From ee80c8c3c06f3a49c86a201359e4d99f8fa3ffa4 Mon Sep 17 00:00:00 2001 From: scottmk Date: Thu, 30 Apr 2009 19:32:00 +0000 Subject: [PATCH] Enforce the requirement that the ORDER BY clause be represented by a JSON_HASH. The old code would often silently ignore a malformed ORDER BY clause. (Note: the output of diff makes this change look spectacularly more complicated than it really is. Because I increased the level of indentation of a large chunk of code, diff made a lot of spurious matches.) git-svn-id: svn://svn.open-ils.org/ILS/trunk@13023 dcc99617-32d9-48b4-a31d-7c20da2025e4 --- Open-ILS/src/c-apps/oils_cstore.c | 423 ++++++++++++++++++++------------------ 1 file changed, 224 insertions(+), 199 deletions(-) diff --git a/Open-ILS/src/c-apps/oils_cstore.c b/Open-ILS/src/c-apps/oils_cstore.c index 177a5bd8aa..46d7d994cf 100644 --- a/Open-ILS/src/c-apps/oils_cstore.c +++ b/Open-ILS/src/c-apps/oils_cstore.c @@ -3369,36 +3369,8 @@ char* SELECT ( "osrfMethodException", ctx->request, "Severe query error in HAVING predicate -- see error log for more details" - ); - } - free(core_class); - buffer_free(having_buf); - buffer_free(group_buf); - buffer_free(sql_buf); - if (defaultselhash) jsonObjectFree(defaultselhash); - return NULL; - } - } - - growing_buffer* order_buf = NULL; // to collect ORDER BY list - - // Build an ORDER BY clause, if there is one - jsonIterator* class_itr = jsonNewIterator( order_hash ); - while ( (snode = jsonIteratorNext( class_itr )) ) { - - if (!jsonObjectGetKeyConst(selhash,class_itr->key)) { - osrfLogError(OSRF_LOG_MARK, "%s: Invalid class \"%s\" referenced in ORDER BY clause", - MODULENAME, class_itr->key ); - if( ctx ) - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Invalid class referenced in ORDER BY clause -- see error log for more details" ); - jsonIteratorFree( class_itr ); - buffer_free( order_buf ); + } free(core_class); buffer_free(having_buf); buffer_free(group_buf); @@ -3406,91 +3378,133 @@ char* SELECT ( if (defaultselhash) jsonObjectFree(defaultselhash); return NULL; } + } - osrfHash* field_list_def = oilsIDLFindPath( "/%s/fields", class_itr->key ); + growing_buffer* order_buf = NULL; // to collect ORDER BY list - if ( snode->type == JSON_HASH ) { + // Build an ORDER BY clause, if there is one + if( NULL == order_hash ) + ; // No ORDER BY? do nothing + else if( JSON_HASH == order_hash->type ) + { + jsonIterator* class_itr = jsonNewIterator( order_hash ); + while ( (snode = jsonIteratorNext( class_itr )) ) { - jsonIterator* order_itr = jsonNewIterator( snode ); - while ( (onode = jsonIteratorNext( order_itr )) ) { + if (!jsonObjectGetKeyConst(selhash,class_itr->key)) { + osrfLogError(OSRF_LOG_MARK, "%s: Invalid class \"%s\" referenced in ORDER BY clause", + MODULENAME, class_itr->key ); + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Invalid class referenced in ORDER BY clause -- see error log for more details" + ); + jsonIteratorFree( class_itr ); + buffer_free( order_buf ); + free(core_class); + buffer_free(having_buf); + buffer_free(group_buf); + buffer_free(sql_buf); + if (defaultselhash) jsonObjectFree(defaultselhash); + return NULL; + } - osrfHash* field_def = osrfHashGet( field_list_def, order_itr->key ); - if( !field_def ) { - osrfLogError(OSRF_LOG_MARK, "%s: Invalid field \"%s\" in ORDER BY clause", - MODULENAME, order_itr->key ); - if( ctx ) - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Invalid field in ORDER BY clause -- see error log for more details" - ); - jsonIteratorFree( order_itr ); - jsonIteratorFree( class_itr ); - buffer_free( order_buf ); - free(core_class); - buffer_free(having_buf); - buffer_free(group_buf); - buffer_free(sql_buf); - if (defaultselhash) jsonObjectFree(defaultselhash); - return NULL; - } else if( str_is_true( osrfHashGet( field_def, "virtual" ) ) ) { - osrfLogError(OSRF_LOG_MARK, "%s: Virtual field \"%s\" in ORDER BY clause", - MODULENAME, order_itr->key ); - if( ctx ) - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Virtual field in ORDER BY clause -- see error log for more details" - ); - jsonIteratorFree( order_itr ); - jsonIteratorFree( class_itr ); - buffer_free( order_buf ); - free(core_class); - buffer_free(having_buf); - buffer_free(group_buf); - buffer_free(sql_buf); - if (defaultselhash) jsonObjectFree(defaultselhash); - return NULL; - } + osrfHash* field_list_def = oilsIDLFindPath( "/%s/fields", class_itr->key ); - const char* direction = NULL; - if ( onode->type == JSON_HASH ) { - if ( jsonObjectGetKeyConst( onode, "transform" ) ) { - string = searchFieldTransform( - class_itr->key, - oilsIDLFindPath( "/%s/fields/%s", class_itr->key, order_itr->key ), - onode - ); - if( ! string ) { - if( ctx ) osrfAppSessionStatus( + if ( snode->type == JSON_HASH ) { + + jsonIterator* order_itr = jsonNewIterator( snode ); + while ( (onode = jsonIteratorNext( order_itr )) ) { + + osrfHash* field_def = osrfHashGet( field_list_def, order_itr->key ); + if( !field_def ) { + osrfLogError(OSRF_LOG_MARK, "%s: Invalid field \"%s\" in ORDER BY clause", + MODULENAME, order_itr->key ); + if( ctx ) + osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, "osrfMethodException", ctx->request, - "Severe query error in ORDER BY clause -- see error log for more details" + "Invalid field in ORDER BY clause -- see error log for more details" ); - jsonIteratorFree( order_itr ); - jsonIteratorFree( class_itr ); - free(core_class); - buffer_free(having_buf); - buffer_free(group_buf); - buffer_free(order_buf); - buffer_free(sql_buf); - if (defaultselhash) jsonObjectFree(defaultselhash); - return NULL; - } - } else { - growing_buffer* field_buf = buffer_init(16); - buffer_fadd(field_buf, "\"%s\".%s", class_itr->key, order_itr->key); - string = buffer_release(field_buf); + jsonIteratorFree( order_itr ); + jsonIteratorFree( class_itr ); + buffer_free( order_buf ); + free(core_class); + buffer_free(having_buf); + buffer_free(group_buf); + buffer_free(sql_buf); + if (defaultselhash) jsonObjectFree(defaultselhash); + return NULL; + } else if( str_is_true( osrfHashGet( field_def, "virtual" ) ) ) { + osrfLogError(OSRF_LOG_MARK, "%s: Virtual field \"%s\" in ORDER BY clause", + MODULENAME, order_itr->key ); + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Virtual field in ORDER BY clause -- see error log for more details" + ); + jsonIteratorFree( order_itr ); + jsonIteratorFree( class_itr ); + buffer_free( order_buf ); + free(core_class); + buffer_free(having_buf); + buffer_free(group_buf); + buffer_free(sql_buf); + if (defaultselhash) jsonObjectFree(defaultselhash); + return NULL; } - if ( (tmp_const = jsonObjectGetKeyConst( onode, "direction" )) ) { - const char* dir = jsonObjectGetString(tmp_const); + const char* direction = NULL; + if ( onode->type == JSON_HASH ) { + if ( jsonObjectGetKeyConst( onode, "transform" ) ) { + string = searchFieldTransform( + class_itr->key, + oilsIDLFindPath( "/%s/fields/%s", class_itr->key, order_itr->key ), + onode + ); + if( ! string ) { + if( ctx ) osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Severe query error in ORDER BY clause -- see error log for more details" + ); + jsonIteratorFree( order_itr ); + jsonIteratorFree( class_itr ); + free(core_class); + buffer_free(having_buf); + buffer_free(group_buf); + buffer_free(order_buf); + buffer_free(sql_buf); + if (defaultselhash) jsonObjectFree(defaultselhash); + return NULL; + } + } else { + growing_buffer* field_buf = buffer_init(16); + buffer_fadd(field_buf, "\"%s\".%s", class_itr->key, order_itr->key); + string = buffer_release(field_buf); + } + + if ( (tmp_const = jsonObjectGetKeyConst( onode, "direction" )) ) { + const char* dir = jsonObjectGetString(tmp_const); + if (!strncasecmp(dir, "d", 1)) { + direction = " DESC"; + } else { + direction = " ASC"; + } + } + + } else { + string = strdup(order_itr->key); + const char* dir = jsonObjectGetString(onode); if (!strncasecmp(dir, "d", 1)) { direction = " DESC"; } else { @@ -3498,115 +3512,126 @@ char* SELECT ( } } - } else { - string = strdup(order_itr->key); - const char* dir = jsonObjectGetString(onode); - if (!strncasecmp(dir, "d", 1)) { - direction = " DESC"; - } else { - direction = " ASC"; - } - } - - if ( order_buf ) - buffer_add(order_buf, ", "); - else - order_buf = buffer_init(128); + if ( order_buf ) + buffer_add(order_buf, ", "); + else + order_buf = buffer_init(128); - buffer_add(order_buf, string); - free(string); + buffer_add(order_buf, string); + free(string); - if (direction) { - buffer_add(order_buf, direction); - } + if (direction) { + buffer_add(order_buf, direction); + } - } // end while - // jsonIteratorFree(order_itr); + } // end while + // jsonIteratorFree(order_itr); - } else if ( snode->type == JSON_ARRAY ) { + } else if ( snode->type == JSON_ARRAY ) { - jsonIterator* order_itr = jsonNewIterator( snode ); - while ( (onode = jsonIteratorNext( order_itr )) ) { + jsonIterator* order_itr = jsonNewIterator( snode ); + while ( (onode = jsonIteratorNext( order_itr )) ) { - const char* _f = jsonObjectGetString( onode ); + const char* _f = jsonObjectGetString( onode ); - osrfHash* field_def = osrfHashGet( field_list_def, _f ); - if( !field_def ) { - osrfLogError(OSRF_LOG_MARK, "%s: Invalid field \"%s\" in ORDER BY clause", - MODULENAME, _f ); - if( ctx ) - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Invalid field in ORDER BY clause -- see error log for more details" - ); - jsonIteratorFree( order_itr ); - jsonIteratorFree( class_itr ); - buffer_free( order_buf ); - free(core_class); - buffer_free(having_buf); - buffer_free(group_buf); - buffer_free(sql_buf); - if (defaultselhash) jsonObjectFree(defaultselhash); - return NULL; - } else if( str_is_true( osrfHashGet( field_def, "virtual" ) ) ) { - osrfLogError(OSRF_LOG_MARK, "%s: Virtual field \"%s\" in ORDER BY clause", - MODULENAME, _f ); - if( ctx ) - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Virtual field in ORDER BY clause -- see error log for more details" - ); - jsonIteratorFree( order_itr ); - jsonIteratorFree( class_itr ); - buffer_free( order_buf ); - free(core_class); - buffer_free(having_buf); - buffer_free(group_buf); - buffer_free(sql_buf); - if (defaultselhash) jsonObjectFree(defaultselhash); - return NULL; - } + osrfHash* field_def = osrfHashGet( field_list_def, _f ); + if( !field_def ) { + osrfLogError(OSRF_LOG_MARK, "%s: Invalid field \"%s\" in ORDER BY clause", + MODULENAME, _f ); + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Invalid field in ORDER BY clause -- see error log for more details" + ); + jsonIteratorFree( order_itr ); + jsonIteratorFree( class_itr ); + buffer_free( order_buf ); + free(core_class); + buffer_free(having_buf); + buffer_free(group_buf); + buffer_free(sql_buf); + if (defaultselhash) jsonObjectFree(defaultselhash); + return NULL; + } else if( str_is_true( osrfHashGet( field_def, "virtual" ) ) ) { + osrfLogError(OSRF_LOG_MARK, "%s: Virtual field \"%s\" in ORDER BY clause", + MODULENAME, _f ); + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Virtual field in ORDER BY clause -- see error log for more details" + ); + jsonIteratorFree( order_itr ); + jsonIteratorFree( class_itr ); + buffer_free( order_buf ); + free(core_class); + buffer_free(having_buf); + buffer_free(group_buf); + buffer_free(sql_buf); + if (defaultselhash) jsonObjectFree(defaultselhash); + return NULL; + } - if ( order_buf ) - buffer_add(order_buf, ", "); - else - order_buf = buffer_init(128); + if ( order_buf ) + buffer_add(order_buf, ", "); + else + order_buf = buffer_init(128); - buffer_add(order_buf, _f); + buffer_add(order_buf, _f); - } // end while + } // end while // jsonIteratorFree(order_itr); + + + // IT'S THE OOOOOOOOOOOLD STYLE! + } else { + osrfLogError(OSRF_LOG_MARK, + "%s: Possible SQL injection attempt; direct order by is not allowed", MODULENAME); + if (ctx) { + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Severe query error -- see error log for more details" + ); + } - - // IT'S THE OOOOOOOOOOOLD STYLE! - } else { - osrfLogError(OSRF_LOG_MARK, "%s: Possible SQL injection attempt; direct order by is not allowed", MODULENAME); - if (ctx) { - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Severe query error -- see error log for more details" - ); - } - - free(core_class); - buffer_free(having_buf); - buffer_free(group_buf); - buffer_free(order_buf); - buffer_free(sql_buf); - if (defaultselhash) jsonObjectFree(defaultselhash); - jsonIteratorFree(class_itr); - return NULL; - } - } // end while + free(core_class); + buffer_free(having_buf); + buffer_free(group_buf); + buffer_free(order_buf); + buffer_free(sql_buf); + if (defaultselhash) jsonObjectFree(defaultselhash); + jsonIteratorFree(class_itr); + return NULL; + } + } // end while + } else { + osrfLogError(OSRF_LOG_MARK, + "%s: Malformed ORDER BY clause; expected JSON_HASH, found %s", + MODULENAME, json_type( order_hash->type ) ); + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Malformed ORDER BY clause -- see error log for more details" + ); + buffer_free( order_buf ); + free(core_class); + buffer_free(having_buf); + buffer_free(group_buf); + buffer_free(sql_buf); + if (defaultselhash) jsonObjectFree(defaultselhash); + return NULL; + } // jsonIteratorFree(class_itr); if( order_buf ) -- 2.11.0