Hi all,
Remember that when we fixed CVE-2012-6123, I intended to simplify a few
procedures to prevent similar mistakes in the future? Here's a patch
that does this for get-environment-variable and ##sys#get-argument.
I don't know why these were hand-rolled in C; by my reckoning there's
no good reason to do this. By using the FFI instead, we can reduce code
bloat and make it more maintainable, too.
There's exactly one difference that I've noticed: when passing an object
of an invalid type or a string with a NUL byte in it, you no longer get
an condition object with a location property in its "exn" subcondition.
I think this is not a good enough reason to reject the patch; instead
maybe we can try and see if we can improve the FFI to pass the location
to its argument conversion/checking routines. This would be more
generally useful for user code, as well!
But that can be done later.
Cheers,
Peter
--
http://www.more-magic.net
>From e32be048bfd0d86fcd45a1951239ba801924fcbc Mon Sep 17 00:00:00 2001
From: Peter Bex <peter....@xs4all.nl>
Date: Fri, 1 Mar 2013 20:41:49 +0100
Subject: [PATCH] Simplify ##sys#get-argument and get-environment-variable by
using the FFI instead of hand-rolled C functions; deprecated these underlying
C functions This should make the code easier to understand, more maintainable
and reduce bloat. It is also a more structural approach of preventing
security problems like those fixed by d9f2ad87b42f by using one vetted
underlying system; the FFI conversion procedures.
---
NEWS | 3 +++
chicken.h | 4 ++--
library.scm | 9 +++++++--
runtime.c | 16 ++++++++++------
4 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/NEWS b/NEWS
index a5f9054..03df87c 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,9 @@
- Runtime system
- Special events in poll() are now handled, avoiding hangs in threaded apps.
+- C API
+ - Deprecated C_get_argument[_2] and C_get_environment_variable[_2] functions.
+
4.8.1
- Security fixes
diff --git a/chicken.h b/chicken.h
index 566aad3..2b9030a 100644
--- a/chicken.h
+++ b/chicken.h
@@ -1782,14 +1782,14 @@ C_fctexport void C_ccall C_quotient(C_word c, C_word
closure, C_word k, C_word n
C_fctexport void C_ccall C_number_to_string(C_word c, C_word closure, C_word
k, C_word num, ...) C_noret;
C_fctexport void C_ccall C_fixnum_to_string(C_word c, C_word closure, C_word
k, C_word num) C_noret;
C_fctexport void C_ccall C_get_argv(C_word c, C_word closure, C_word k)
C_noret; /* OBSOLETE */
-C_fctexport void C_ccall C_get_argument(C_word c, C_word closure, C_word k,
C_word index) C_noret;
+C_fctexport void C_ccall C_get_argument(C_word c, C_word closure, C_word k,
C_word index) C_noret; /* OBSOLETE */
C_fctexport void C_ccall C_make_structure(C_word c, C_word closure, C_word k,
C_word type, ...) C_noret;
C_fctexport void C_ccall C_make_symbol(C_word c, C_word closure, C_word k,
C_word name) C_noret;
C_fctexport void C_ccall C_make_pointer(C_word c, C_word closure, C_word k)
C_noret;
C_fctexport void C_ccall C_make_tagged_pointer(C_word c, C_word closure,
C_word k, C_word tag) C_noret;
C_fctexport void C_ccall C_ensure_heap_reserve(C_word c, C_word closure,
C_word k, C_word n) C_noret;
C_fctexport void C_ccall C_return_to_host(C_word c, C_word closure, C_word k)
C_noret;
-C_fctexport void C_ccall C_get_environment_variable(C_word c, C_word closure,
C_word k, C_word name) C_noret;
+C_fctexport void C_ccall C_get_environment_variable(C_word c, C_word closure,
C_word k, C_word name) C_noret; /* OBSOLETE */
C_fctexport void C_ccall C_get_symbol_table_info(C_word c, C_word closure,
C_word k) C_noret;
C_fctexport void C_ccall C_get_memory_info(C_word c, C_word closure, C_word k)
C_noret;
C_fctexport void C_ccall C_context_switch(C_word c, C_word closure, C_word k,
C_word state) C_noret;
diff --git a/library.scm b/library.scm
index 31e1e0c..5029186 100644
--- a/library.scm
+++ b/library.scm
@@ -202,7 +202,7 @@ EOF
(define (current-milliseconds) (##core#inline_allocate
("C_a_i_current_milliseconds" 4) #f))
(define (current-gc-milliseconds) (##sys#fudge 31))
(define ##sys#decode-seconds (##core#primitive "C_decode_seconds"))
-(define get-environment-variable (##core#primitive
"C_get_environment_variable"))
+(define get-environment-variable (foreign-lambda c-string "C_getenv" c-string))
(define (##sys#start-timer)
(##sys#gc #t)
@@ -4594,7 +4594,12 @@ EOF
;;; command-line handling
-(define ##sys#get-argument (##core#primitive "C_get_argument"))
+(define ##sys#get-argument
+ (let ((fetch-arg (foreign-lambda* c-string ((int i))
+ "C_return(C_main_argv[i]);")))
+ (lambda (index)
+ (and (fx< index main_argc)
+ (fetch-arg index)))))
(define argv ; includes program name
(let ((cache #f))
diff --git a/runtime.c b/runtime.c
index a5e45fc..61b4aba 100644
--- a/runtime.c
+++ b/runtime.c
@@ -500,10 +500,10 @@ static C_ccall void call_cc_values_wrapper(C_word c,
C_word closure, C_word k, .
static void gc_2(void *dummy) C_noret;
static void allocate_vector_2(void *dummy) C_noret;
static void get_argv_2(void *dummy) C_noret; /* OBSOLETE */
-static void get_argument_2(void *dummy) C_noret;
+static void get_argument_2(void *dummy) C_noret; /* OBSOLETE */
static void make_structure_2(void *dummy) C_noret;
static void generic_trampoline(void *dummy) C_noret;
-static void get_environment_variable_2(void *dummy) C_noret;
+static void get_environment_variable_2(void *dummy) C_noret; /* OBSOLETE */
static void handle_interrupt(void *trampoline, void *proc) C_noret;
static void callback_trampoline(void *dummy) C_noret;
static C_ccall void callback_return_continuation(C_word c, C_word self, C_word
r) C_noret;
@@ -790,7 +790,7 @@ static C_PTABLE_ENTRY *create_initial_ptable()
C_pte(C_get_symbol_table_info);
C_pte(C_get_memory_info);
C_pte(C_decode_seconds);
- C_pte(C_get_environment_variable);
+ C_pte(C_get_environment_variable); /* OBSOLETE */
C_pte(C_stop_timer);
C_pte(C_dload);
C_pte(C_set_dlopen_flags);
@@ -833,7 +833,7 @@ static C_PTABLE_ENTRY *create_initial_ptable()
C_pte(C_copy_closure);
C_pte(C_dump_heap_state);
C_pte(C_filter_heap_objects);
- C_pte(C_get_argument);
+ C_pte(C_get_argument); /* OBSOLETE */
/* IMPORTANT: did you remember the hardcoded pte table size? */
pt[ i ].id = NULL;
@@ -7822,6 +7822,7 @@ void get_argv_2(void *dummy)
}
+/* OBSOLETE */
void C_ccall C_get_argument(C_word c, C_word closure, C_word k, C_word index)
{
int i = C_unfix(index);
@@ -7841,6 +7842,7 @@ void C_ccall C_get_argument(C_word c, C_word closure,
C_word k, C_word index)
}
+/* OBSOLETE */
void get_argument_2(void *dummy)
{
int i = C_unfix(C_restore);
@@ -7955,10 +7957,11 @@ void C_ccall C_return_to_host(C_word c, C_word closure,
C_word k)
}
-#define C_do_getenv(v) C_getenv(v)
-#define C_free_envbuf() {}
+#define C_do_getenv(v) C_getenv(v) /* OBSOLETE */
+#define C_free_envbuf() {} /* OBSOLETE */
+/* OBSOLETE */
void C_ccall C_get_environment_variable(C_word c, C_word closure, C_word k,
C_word name)
{
int len;
@@ -7989,6 +7992,7 @@ void C_ccall C_get_environment_variable(C_word c, C_word
closure, C_word k, C_wo
}
+/* OBSOLETE */
void get_environment_variable_2(void *dummy)
{
int len = C_strlen(save_string);
--
1.8.0.1
_______________________________________________
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers
