From e99858671db19e07e61224757ff9f8bc14706c8c Mon Sep 17 00:00:00 2001 From: scottmk Date: Tue, 7 Apr 2009 13:12:36 +0000 Subject: [PATCH] In oils_cstore.c: 1. Check the return code from all calls to SELECT(). 2. In searchValueTransform(): insist that the function name be in a JSON_STRING. 3. In searchFieldTransformPredicate(): screen the operator string for SQL injection. 4. In searchFieldTransformPredicate(): insert another layer of parentheses to avoid ambiguity. 5. Eliminated extraneous parameters on some calls to osrfLogError(). 6. In searchFieldTransformPredicate(): disallow comparisons to nulls and booleans. git-svn-id: svn://svn.open-ils.org/ILS/trunk@12810 dcc99617-32d9-48b4-a31d-7c20da2025e4 --- Open-ILS/src/c-apps/oils_cstore.c | 96 +++++++++++++++++++++++-------- 1 file changed, 73 insertions(+), 23 deletions(-) diff --git a/Open-ILS/src/c-apps/oils_cstore.c b/Open-ILS/src/c-apps/oils_cstore.c index 379c24ccb3..afc4ad47b1 100644 --- a/Open-ILS/src/c-apps/oils_cstore.c +++ b/Open-ILS/src/c-apps/oils_cstore.c @@ -1681,8 +1681,13 @@ static char* searchINPredicate (const char* class, osrfHash* field, SUBSELECT ); - buffer_add(sql_buf, subpred); - free(subpred); + if( subpred ) { + buffer_add(sql_buf, subpred); + free(subpred); + } else { + buffer_free( sql_buf ); + return NULL; + } } else if (node->type == JSON_ARRAY) { // literal value list @@ -1755,10 +1760,16 @@ static char* searchValueTransform( const jsonObject* array ) { return NULL; } - growing_buffer* sql_buf = buffer_init(32); - // Get the function name jsonObject* func_item = jsonObjectGetIndex( array, 0 ); + if( func_item->type != JSON_STRING ) { + osrfLogError(OSRF_LOG_MARK, "%s: Error: expected function name, found %s", + MODULENAME, json_type( func_item->type ) ); + return NULL; + } + + growing_buffer* sql_buf = buffer_init(32); + OSRF_BUFFER_ADD( sql_buf, jsonObjectGetString( func_item ) ); OSRF_BUFFER_ADD( sql_buf, "( " ); @@ -1821,7 +1832,7 @@ static char* searchFunctionPredicate (const char* class, osrfHash* field, // class is a class name // field is a field definition as stored in the IDL -// node comes from the method parameter, and represents an entry in the SELECT list +// node comes from the method parameter, and may represent an entry in the SELECT list static char* searchFieldTransform (const char* class, osrfHash* field, const jsonObject* node) { growing_buffer* sql_buf = buffer_init(32); @@ -1876,37 +1887,53 @@ static char* searchFieldTransform (const char* class, osrfHash* field, const jso return buffer_release(sql_buf); } -static char* searchFieldTransformPredicate (const char* class, osrfHash* field, - const jsonObject* node, const char* node_key) { +static char* searchFieldTransformPredicate (const char* class, osrfHash* field, + const jsonObject* node, const char* op ) { + + if( ! is_good_operator( op ) ) { + osrfLogError(OSRF_LOG_MARK, "%s: Error: Invalid operator %s", MODULENAME, op); + return NULL; + } + char* field_transform = searchFieldTransform( class, field, node ); char* value = NULL; + int extra_parens = 0; // boolean const jsonObject* value_obj = jsonObjectGetKeyConst( node, "value" ); if ( ! value_obj ) { value = searchWHERE( node, osrfHashGet( oilsIDL(), class ), AND_OP_JOIN, NULL ); if( !value ) { - osrfLogError(OSRF_LOG_MARK, "%s: Error building condition for field transform", MODULENAME, value); + osrfLogError(OSRF_LOG_MARK, "%s: Error building condition for field transform", MODULENAME); free(field_transform); return NULL; } + extra_parens = 1; } else if ( value_obj->type == JSON_ARRAY ) { value = searchValueTransform( value_obj ); if( !value ) { - osrfLogError(OSRF_LOG_MARK, "%s: Error building value transform for field transform", - MODULENAME, value); + osrfLogError(OSRF_LOG_MARK, "%s: Error building value transform for field transform", MODULENAME); free( field_transform ); return NULL; } } else if ( value_obj->type == JSON_HASH ) { value = searchWHERE( value_obj, osrfHashGet( oilsIDL(), class ), AND_OP_JOIN, NULL ); if( !value ) { - osrfLogError(OSRF_LOG_MARK, "%s: Error building predicate for field transform", MODULENAME, value); + osrfLogError(OSRF_LOG_MARK, "%s: Error building predicate for field transform", MODULENAME); free(field_transform); return NULL; } + extra_parens = 1; } else if ( value_obj->type == JSON_NUMBER ) { value = jsonNumberToDBString( field, value_obj ); - } else if ( value_obj->type != JSON_NULL ) { + } else if ( value_obj->type == JSON_NULL ) { + osrfLogError(OSRF_LOG_MARK, "%s: Error building predicate for field transform: null value", MODULENAME); + free(field_transform); + return NULL; + } else if ( value_obj->type == JSON_BOOL ) { + osrfLogError(OSRF_LOG_MARK, "%s: Error building predicate for field transform: boolean value", MODULENAME); + free(field_transform); + return NULL; + } else { if ( !strcmp( get_primitive( field ), "number") ) { value = jsonNumberToDBString( field, value_obj ); } else { @@ -1920,14 +1947,26 @@ static char* searchFieldTransformPredicate (const char* class, osrfHash* field, } } + const char* left_parens = ""; + const char* right_parens = ""; + + if( extra_parens ) { + left_parens = "("; + right_parens = ")"; + } + growing_buffer* sql_buf = buffer_init(32); - + buffer_fadd( sql_buf, - "%s %s %s", + "%s%s %s %s %s %s%s", + left_parens, field_transform, - node_key, - value + op, + left_parens, + value, + right_parens, + right_parens ); free(value); @@ -2031,7 +2070,7 @@ static char* searchPredicate ( const char* class, osrfHash* field, char* pred = NULL; if (node->type == JSON_ARRAY) { // equality IN search pred = searchINPredicate( class, field, node, NULL, ctx ); - } else if (node->type == JSON_HASH) { // non-equality search + } else if (node->type == JSON_HASH) { // other search jsonIterator* pred_itr = jsonNewIterator( node ); if( !jsonIteratorHasNext( pred_itr ) ) { osrfLogError( OSRF_LOG_MARK, "%s: Empty predicate for field \"%s\"", @@ -2493,8 +2532,13 @@ static char* searchWHERE ( const jsonObject* search_hash, osrfHash* meta, int op SUBSELECT ); - buffer_fadd(sql_buf, "EXISTS ( %s )", subpred); - free(subpred); + if( subpred ) { + buffer_fadd(sql_buf, "EXISTS ( %s )", subpred); + free(subpred); + } else { + buffer_free( sql_buf ); + return NULL; + } } else if ( !strcasecmp("-not-exists",search_itr->key) ) { char* subpred = SELECT( ctx, @@ -2508,8 +2552,14 @@ static char* searchWHERE ( const jsonObject* search_hash, osrfHash* meta, int op SUBSELECT ); - buffer_fadd(sql_buf, "NOT EXISTS ( %s )", subpred); - free(subpred); + if( subpred ) { + buffer_fadd(sql_buf, "NOT EXISTS ( %s )", subpred); + free(subpred); + } else { + buffer_free( sql_buf ); + return NULL; + } + } else { char* class = osrfHashGet(meta, "classname"); @@ -2523,11 +2573,11 @@ static char* searchWHERE ( const jsonObject* search_hash, osrfHash* meta, int op table = strdup( "(?)" ); osrfLogError( OSRF_LOG_MARK, - "%s: Attempt to reference non-existent column %s on %s (%s)", + "%s: Attempt to reference non-existent column \"%s\" on %s (%s)", MODULENAME, search_itr->key, table, - class + class ? class : "?" ); buffer_free(sql_buf); free(table); -- 2.43.2