>From 643848bbad2cb8858030507ca1c26bb0817b3633 Mon Sep 17 00:00:00 2001 From: Christian Kellermann Date: Sat, 23 Jul 2016 21:23:50 +0200 Subject: [PATCH] Fix buffer overflow in posix execvp/execve wrapper This fixes bug #1308 found by wasamasa. It turns out that we don't check the number of arguments or the number of env entries before trying to write them to the target string. That target string is only ARG_MAX or ENV_MAX bytes long. This patch adds a ##sys#check-range for both arguments. The unix implementation and the windows implementation are both affected and fixed with this patch. --- posixunix.scm | 6 +++++- posixwin.scm | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/posixunix.scm b/posixunix.scm index a21d0b0..cc9c947 100644 --- a/posixunix.scm +++ b/posixunix.scm @@ -1599,10 +1599,13 @@ EOF [freeargs (foreign-lambda void "C_free_exec_args")] [setenv (foreign-lambda void "C_set_exec_env" int c-string int)] [freeenv (foreign-lambda void "C_free_exec_env")] - [pathname-strip-directory pathname-strip-directory] ) + [pathname-strip-directory pathname-strip-directory] + [arg-max (foreign-value "ARG_MAX" size_t)] + [env-max (foreign-value "ENV_MAX" size_t)] ) (lambda (filename #!optional (arglist '()) envlist) (##sys#check-string filename 'process-execute) (##sys#check-list arglist 'process-execute) + (##sys#check-range (##sys#length arglist) 0 arg-max 'process-execute) (let ([s (pathname-strip-directory filename)]) (setarg 0 s (##sys#size s)) ) (do ([al arglist (cdr al)] @@ -1611,6 +1614,7 @@ EOF (setarg i #f 0) (when envlist (##sys#check-list envlist 'process-execute) + (##sys#check-range (##sys#length envlist) 0 env-max 'process-execute) (do ([el envlist (cdr el)] [i 0 (fx+ i 1)] ) ((null? el) (setenv i #f 0)) diff --git a/posixwin.scm b/posixwin.scm index 2f46aaf..cc9a583 100644 --- a/posixwin.scm +++ b/posixwin.scm @@ -1191,11 +1191,14 @@ EOF ;; At least it's secure, we can worry about performance later, if at all (let ([setarg (foreign-lambda void "C_set_exec_arg" int c-string int)] [setenv (foreign-lambda void "C_set_exec_env" int c-string int)] + [arg-max (foreign-value "ARG_MAX" size_t)] + [env-max (foreign-value "ENV_MAX" size_t)] [build-exec-argvec - (lambda (loc lst argvec-setter idx) + (lambda (loc lst argvec-setter idx max) (if lst (begin (##sys#check-list lst loc) + (##sys-check-range (##sys#length lst) 0 max loc) (do ([l lst (cdr l)] [i idx (fx+ i 1)] ) ((null? l) (argvec-setter i #f 0)) @@ -1207,8 +1210,8 @@ EOF (##sys#check-string filename loc) (let ([s (pathname-strip-directory filename)]) (setarg 0 s (##sys#size s)) ) - (build-exec-argvec loc (and arglst ($quote-args-list arglst exactf)) setarg 1) - (build-exec-argvec loc envlst setenv 0) + (build-exec-argvec loc (and arglst ($quote-args-list arglst exactf)) setarg 1 arg-max) + (build-exec-argvec loc envlst setenv 0 env-max) (##core#inline "C_flushall") (##sys#make-c-string filename loc) ) ) ) -- 2.9.0