security: close plugin-gate bypass via project-config opt-in (P0 RCE)
ober
8c1fcfa13d1182b22c9c79c0b9c7eeb376806ee6
--- a/src/jcode/core/config.ss +++ b/src/jcode/core/config.ss @@ -3,6 +3,7 @@ (export load-config jcode-home config-ref + config-global-ref config-api-key config-model config-provider @@ -127,6 +128,23 @@ ((not (hash-table? obj)) #f) (else (loop (hash-get obj (car keys)) (cdr keys)))))) +(def (config-global-ref . keys) + ;; Read a key path from ONLY the trusted global config (~/.jcode/config.json), + ;; ignoring the project-local ./jcode.json that load-config deep-merges on top. + ;; Security-sensitive opt-ins (e.g. pluginAllowWorkspace, pluginDirs) must + ;; resolve here: a cloned repository ships ./jcode.json and would otherwise be + ;; able to set them itself, turning an "explicit user opt-in" into an + ;; attacker-controlled value. + (let ((cfg (guard (e [else #f]) + (let ((p (path-join (jcode-home) "config.json"))) + (and (file-exists? p) + (call-with-input-file p read-json)))))) + (let loop ((obj cfg) (ks keys)) + (cond + ((null? ks) obj) + ((not (hash-table? obj)) #f) + (else (loop (hash-get obj (car ks)) (cdr ks))))))) + (def (env-nonempty var) (let ((v (getenv var))) (and v (not (string=? v "")) v))) --- a/src/jcode/core/plugin.ss +++ b/src/jcode/core/plugin.ss @@ -19,39 +19,35 @@ "Workspace-local plugins (<cwd>/.jcode/plugins) are untrusted: a cloned repository could ship one and obtain silent code execution at startup. They are disabled unless the user explicitly opts in via the - JCODE_ALLOW_WORKSPACE_PLUGINS env var or the pluginAllowWorkspace config." + JCODE_ALLOW_WORKSPACE_PLUGINS env var or the pluginAllowWorkspace setting in + the TRUSTED global config (~/.jcode/config.json). The opt-in is resolved via + config-global-ref on purpose: a project-local ./jcode.json is itself + attacker-controllable, so honoring pluginAllowWorkspace from the merged + config would let a cloned repo grant itself the opt-in and bypass this gate." (or (let ((v (getenv "JCODE_ALLOW_WORKSPACE_PLUGINS"))) (and v (not (string=? v "")) (not (string=? v "0")))) - (config-ref "pluginAllowWorkspace"))) - -(def (string-prefix-safe? prefix s) - (and (>= (string-length s) (string-length prefix)) - (string=? prefix (substring s 0 (string-length prefix))))) - -(def (workspace-relative-dir? dir) - "True when DIR resolves to a location inside the current workspace (cwd). - A project-local ./jcode.json is attacker-controllable (a cloned repo could - ship one), so any configured pluginDir that lives in the workspace is - untrusted code and must obey the same opt-in gate as <cwd>/.jcode/plugins. - Absolute dirs outside the workspace come from the user's global config and - remain trusted." - (and (string? dir) - (let* ((cwd (current-directory)) - (abs (if (string-prefix-safe? "/" dir) dir (path-join cwd dir))) - (cwd-slash (string-append cwd "/"))) - (or (string=? abs cwd) - (string-prefix-safe? cwd-slash abs))))) + (config-global-ref "pluginAllowWorkspace"))) (def (plugin-dirs) "Return list of directories to scan for plugins." (let* ((home-dir (path-join (jcode-home) "plugins")) (local-dir (path-join (current-directory) ".jcode" "plugins")) (cfg-dirs (config-ref "pluginDirs")) - (allow-ws (workspace-plugins-allowed?)) (cfg-dirs (if (and cfg-dirs (list? cfg-dirs)) cfg-dirs '())) - ;; Gate workspace-local configured dirs exactly like local-dir. + ;; pluginDirs named in the TRUSTED global config only. A configured + ;; dir is loaded without opt-in ONLY when it appears here; any other + ;; configured dir comes from the attacker-controllable project + ;; ./jcode.json (which deep-merges over the global config) and must + ;; obey the same opt-in gate as <cwd>/.jcode/plugins. Checking global + ;; membership rather than asking whether the path is absolute closes + ;; the hole where a project file named an absolute dir and had it + ;; trusted on the false assumption that absolute paths only ever come + ;; from the global config. + (global-dirs (let ((d (config-global-ref "pluginDirs"))) + (if (and d (list? d)) d '()))) + (allow-ws (workspace-plugins-allowed?)) (safe-cfg (filter (lambda (d) - (or allow-ws (not (workspace-relative-dir? d)))) + (or allow-ws (member d global-dirs))) cfg-dirs))) (unless allow-ws (when (file-directory? local-dir) @@ -60,7 +56,7 @@ (hint . "set JCODE_ALLOW_WORKSPACE_PLUGINS=1 to load workspace-local plugins")))) (unless (= (length safe-cfg) (length cfg-dirs)) (log-warn logger "workspace-pluginDirs-skipped" - `((hint . "a project pluginDir resolves inside the workspace; set JCODE_ALLOW_WORKSPACE_PLUGINS=1 to load it"))))) + `((hint . "a project pluginDir is not from the trusted global config; set JCODE_ALLOW_WORKSPACE_PLUGINS=1 to load it"))))) (filter file-directory? (append safe-cfg --- a/test/security-regression.ss +++ b/test/security-regression.ss @@ -217,6 +217,68 @@ (when (file-exists? pd-marker) (delete-file pd-marker)) (system (string-append "rm -rf '" pd-home "'")) +;; (a) extended again: the opt-in flag itself must not be settable from the +;; attacker-controllable project config. load-config deep-merges ./jcode.json +;; over the global config, so a cloned repo shipping {"pluginAllowWorkspace":true} +;; would otherwise grant itself the opt-in and re-enable workspace-plugin +;; autoload RCE. The flag (and pluginDirs trust) must resolve from the trusted +;; global config (~/.jcode/config.json) only. +(define pw-dir (string-append (current-directory) "/.jcode/plugins")) +(define pw-evil (string-append pw-dir "/evil.ss")) +(define pw-marker "/tmp/jcode-sec-pw-marker") +(define pw-home "/tmp/jcode-sec-pw-home") +(define pw-abs-dir "/tmp/jcode-sec-pw-abs-plugins") +(define pw-abs-evil (string-append pw-abs-dir "/evil.ss")) +(define pw-saved-home (getenv "HOME")) +(define pw-saved-config (*config*)) +(define pw-saved-opt (getenv "JCODE_ALLOW_WORKSPACE_PLUGINS")) +(define pw-had-dir (file-directory? pw-dir)) +(system (string-append "mkdir -p '" pw-home "/.jcode' '" pw-abs-dir "'")) +(unless pw-had-dir (mkdir pw-dir)) +(for-each (lambda (f) (when (file-exists? f) (delete-file f))) + (list pw-evil pw-abs-evil pw-marker)) +(let ((payload (lambda (p) + (display (string-append "(call-with-output-file \"" pw-marker + "\" (lambda (pp) (display \"pwned\" pp)))") p)))) + (call-with-output-file pw-evil payload) + (call-with-output-file pw-abs-evil payload)) +(putenv "HOME" pw-home) +(putenv "JCODE_ALLOW_WORKSPACE_PLUGINS" "0") + +;; project (merged) config sets the opt-in flag -> must NOT enable autoload +(let ((cfg (jh:make-hash-table))) + (jh:hash-put! cfg "pluginAllowWorkspace" #t) + (*config* cfg)) +(init-plugins) +(check "project-config pluginAllowWorkspace does NOT enable autoload (no RCE)" + (not (file-exists? pw-marker))) + +;; trusted global config file sets the opt-in flag -> autoload still works +(call-with-output-file (string-append pw-home "/.jcode/config.json") + (lambda (p) (display "{\"pluginAllowWorkspace\": true}" p))) +(*config* (jh:make-hash-table)) +(init-plugins) +(check "global-config pluginAllowWorkspace enables autoload (legit opt-in)" + (file-exists? pw-marker)) +(when (file-exists? pw-marker) (delete-file pw-marker)) +(delete-file (string-append pw-home "/.jcode/config.json")) + +;; absolute pluginDir named only in project (merged) config -> NOT trusted +(let ((cfg (jh:make-hash-table))) + (jh:hash-put! cfg "pluginDirs" (list pw-abs-dir)) + (*config* cfg)) +(init-plugins) +(check "absolute pluginDir from project config NOT auto-loaded (no RCE)" + (not (file-exists? pw-marker))) + +(*config* pw-saved-config) +(putenv "HOME" pw-saved-home) +(putenv "JCODE_ALLOW_WORKSPACE_PLUGINS" (or pw-saved-opt "0")) +(for-each (lambda (f) (when (file-exists? f) (delete-file f))) + (list pw-evil pw-abs-evil pw-marker)) +(unless pw-had-dir (system (string-append "rmdir '" pw-dir "'"))) +(system (string-append "rm -rf '" pw-home "' '" pw-abs-dir "'")) + ;; ── (b) mentions command injection + policy bypass ──────────────────── (define diff-probe "/tmp/jcode-sec-diff-probe") (when (file-exists? diff-probe) (delete-file diff-probe))