fixup! src,lib: initial ffi implementation

Author: bengl

doc/api/cli.md

@@ -141,6 +141,48 @@ Error: Access to this API has been restricted
 }
 ```
 
+### `--allow-ffi`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+When using the [Permission Model][], the process will not be able to get
+pointers from buffers, or load shared libraries through the `node:ffi` module by
+default. Attempts to do so will throw an `ERR_ACCESS_DENIED` unless the
+user explicitly passes the `--allow-ffi` flag when starting Node.js.
+
+Example:
+
+```js
+const ffi = require('node:ffi');
+// Attempt to bypass the permission
+ffi.getBufferPointer(Buffer.alloc(1));
+```
+
+```console
+$ node --experimental-permission --allow-fs-read=* index.js
+node:ffi:226
+  const ptr = getBufferPointerInternal(buf);
+              ^
+
+Error: Access to this API has been restricted
+    at Object.getBufferPointer (node:ffi:226:15)
+    at Object.<anonymous> (/Users/bryan.english/node/script.js:3:5)
+    at Module._compile (node:internal/modules/cjs/loader:1287:14)
+    at Module._extensions..js (node:internal/modules/cjs/loader:1341:10)
+    at Module.load (node:internal/modules/cjs/loader:1145:32)
+    at Module._load (node:internal/modules/cjs/loader:984:12)
+    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
+    at node:internal/main/run_main_module:23:47 {
+  code: 'ERR_ACCESS_DENIED',
+  permission: 'Ffi',
+  resource: ''
+}
+```
+
 ### `--allow-fs-read`
 
 <!-- YAML

doc/api/permissions.md

@@ -464,9 +464,9 @@ will restrict access to all available permissions.
 The available permissions are documented by the [`--experimental-permission`][]
 flag.
 
-When starting Node.js with `--experimental-permission`,
-the ability to access the file system, spawn processes, and
-use `node:worker_threads` will be restricted.
+When starting Node.js with `--experimental-permission`, the ability to access
+the file system, spawn processes, use `node:worker_threads`, and load shared
+libraries with `node:ffi` will be restricted.
 
 ```console
 $ node --experimental-permission index.js
@@ -485,8 +485,9 @@ Error: Access to this API has been restricted
 }
 ```
 
-Allowing access to spawning a process and creating worker threads can be done
-using the [`--allow-child-process`][] and [`--allow-worker`][] respectively.
+Allowing access to spawning a process, creating worker threads, and loading
+shared libraries with FFI can be done using the [`--allow-child-process`][],
+[`--allow-worker`][], and [`--allow-ffi`][] flags respectively.
 
 #### Runtime API
 
@@ -588,6 +589,7 @@ const fd = fs.openSync('./README.md', 'r');
 
 [Security Policy]: https://github.com/nodejs/node/blob/main/SECURITY.md
 [`--allow-child-process`]: cli.md#--allow-child-process
+[`--allow-ffi`]: cli.md#--allow-ffi
 [`--allow-fs-read`]: cli.md#--allow-fs-read
 [`--allow-fs-write`]: cli.md#--allow-fs-write
 [`--allow-worker`]: cli.md#--allow-worker

doc/node.1

@@ -82,6 +82,9 @@ Allow file system read access when using the permission model.
 .It Fl -allow-fs-write
 Allow file system write access when using the permission model.
 .
+.It Fl -allow-ffi
+Allow foreign function interface (FFI) when using the permission model.
+.
 .It Fl -allow-child-process
 Allow spawning process when using the permission model.
 .

lib/internal/process/pre_execution.js

@@ -512,6 +512,7 @@ function initializePermission() {
     const { has, deny } = require('internal/process/permission');
     const warnFlags = [
       '--allow-child-process',
+      '--allow-ffi',
       '--allow-worker',
     ];
     for (const flag of warnFlags) {
@@ -537,6 +538,7 @@ function initializePermission() {
       '--allow-fs-write',
       '--allow-child-process',
       '--allow-worker',
+      '--allow-ffi',
     ];
     ArrayPrototypeForEach(availablePermissionFlags, (flag) => {
       if (getOptionValue(flag)) {

node.gyp

@@ -547,6 +547,7 @@
         'src/node_worker.cc',
         'src/node_zlib.cc',
         'src/permission/child_process_permission.cc',
+        'src/permission/ffi_permission.cc',
         'src/permission/fs_permission.cc',
         'src/permission/permission.cc',
         'src/permission/worker_permission.cc',
@@ -663,6 +664,7 @@
         'src/node_watchdog.h',
         'src/node_worker.h',
         'src/permission/child_process_permission.h',
+        'src/permission/ffi_permission.h',
         'src/permission/fs_permission.h',
         'src/permission/permission.h',
         'src/permission/permission_node.h',

src/env.cc

@@ -769,6 +769,9 @@ Environment::Environment(IsolateData* isolate_data,
       if (!options_->allow_worker_threads) {
         permission()->Deny(permission::PermissionScope::kWorkerThreads, {});
       }
+      if (!options_->allow_ffi) {
+        permission()->Deny(permission::PermissionScope::kFfi, {});
+      }
     }
 
     if (!options_->allow_fs_read.empty()) {

src/node_ffi.cc

@@ -30,6 +30,8 @@ static bool lossless = false;
 
 void GetLibrary(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFfi, "");
 
   bool isNull = true;
   std::string fname = "";
@@ -59,6 +61,9 @@ void GetLibrary(const FunctionCallbackInfo<Value>& args) {
 
 void GetSymbol(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFfi, "");
+
   CHECK(args[0]->IsBigInt());
   binding::DLib * lib = (binding::DLib *)args[0].As<BigInt>()
     ->Uint64Value(&lossless);
@@ -69,6 +74,10 @@ void GetSymbol(const FunctionCallbackInfo<Value>& args) {
 }
 
 void GetBufferPointer(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFfi, "");
+
   CHECK(args[0]->IsArrayBuffer());
   void* data = args[0].As<ArrayBuffer>()->Data();
 
@@ -116,12 +125,16 @@ FfiSignature::~FfiSignature() {
 }
 
 void FfiSignature::New(const FunctionCallbackInfo<Value>& args) {
+  Isolate * isolate = args.GetIsolate();
+  Environment * env = Environment::GetCurrent(isolate);
+
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFfi, "");
+
   CHECK(args.IsConstructCall());
   CHECK(args[0]->IsBigInt());
   CHECK(args[1]->IsBigInt());
   CHECK(args[2]->IsArray());
-  Isolate * isolate = args.GetIsolate();
-  Environment * env = Environment::GetCurrent(isolate);
 
   FfiSignature* sig = new FfiSignature(
       env,
@@ -137,6 +150,11 @@ void FfiSignature::New(const FunctionCallbackInfo<Value>& args) {
 }
 
 void MakeCall(const FunctionCallbackInfo<Value> &args) {
+  Isolate * isolate = args.GetIsolate();
+  Environment * env = Environment::GetCurrent(isolate);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFfi, "");
+
   char* callBuffer = reinterpret_cast<char*>(
       Realm::GetBindingData<FfiBindingData>(args)->callBuffer);
   FfiSignature * sig = *reinterpret_cast<FfiSignature **>(callBuffer);

src/node_options.cc

@@ -436,6 +436,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "allow worker threads when any permissions are set",
             &EnvironmentOptions::allow_worker_threads,
             kAllowedInEnvvar);
+  AddOption("--allow-ffi",
+            "allow FFI when any permissions are set",
+            &EnvironmentOptions::allow_ffi,
+            kAllowedInEnvvar);
   AddOption("--experimental-repl-await",
             "experimental await keyword support in REPL",
             &EnvironmentOptions::experimental_repl_await,

src/node_options.h

@@ -123,6 +123,7 @@ class EnvironmentOptions : public Options {
   bool experimental_permission = false;
   std::string allow_fs_read;
   std::string allow_fs_write;
+  bool allow_ffi = false;
   bool allow_child_process = false;
   bool allow_worker_threads = false;
   bool experimental_repl_await = true;

src/permission/ffi_permission.cc

@@ -0,0 +1,26 @@
+#include "permission/ffi_permission.h"
+
+#include <string>
+#include <vector>
+
+namespace node {
+
+namespace permission {
+
+// Currently, PolicyDenyFfi manage a single state
+// Once denied, it's always denied
+void FfiPermission::Apply(const std::string& deny, PermissionScope scope) {}
+
+bool FfiPermission::Deny(PermissionScope perm,
+                            const std::vector<std::string>& params) {
+  deny_all_ = true;
+  return true;
+}
+
+bool FfiPermission::is_granted(PermissionScope perm,
+                                  const std::string_view& param) {
+  return deny_all_ == false;
+}
+
+}  // namespace permission
+}  // namespace node