From c94c559a17322a464cd6f9096942ec0a8585f65c Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Fri, 11 Jan 2013 02:30:50 -0500 Subject: [PATCH] LP#1098377: sanitize savepoint names When invoking open-ils.{cstore,pcrud,rstore}.savepoint.*, the caller supplies a name for the savepoint. However, the savepoint names could be constructed so that the caller could execute arbitrary SQL. This patch sanitizes the name so that it contains only alphanumeric and underscore characters. Signed-off-by: Galen Charlton Signed-off-by: Dan Scott Conflicts: Open-ILS/src/c-apps/oils_sql.c --- Open-ILS/src/c-apps/oils_sql.c | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c index 2f19ddb48e..c7b830ccf0 100644 --- a/Open-ILS/src/c-apps/oils_sql.c +++ b/Open-ILS/src/c-apps/oils_sql.c @@ -143,6 +143,8 @@ static int perm_at_threshold = 5; static int enforce_pcrud = 0; // Boolean static char* modulename = NULL; +static char* _sanitize_savepoint_name( const char* sp ); + /** @brief Connect to the database. @return A database connection if successful, or NULL if not. @@ -872,8 +874,10 @@ int setSavepoint( osrfMethodContext* ctx ) { // Get the savepoint name from the method params const char* spName = jsonObjectGetString( jsonObjectGetIndex(ctx->params, spNamePos) ); + char *safeSpName = _sanitize_savepoint_name( spName ); - dbi_result result = dbi_conn_queryf( writehandle, "SAVEPOINT \"%s\";", spName ); + dbi_result result = dbi_conn_queryf( writehandle, "SAVEPOINT \"%s\";", safeSpName ); + free( safeSpName ); if( !result ) { const char* msg; int errnum = dbi_conn_error( writehandle, &msg ); @@ -943,8 +947,10 @@ int releaseSavepoint( osrfMethodContext* ctx ) { // Get the savepoint name from the method params const char* spName = jsonObjectGetString( jsonObjectGetIndex(ctx->params, spNamePos) ); + char *safeSpName = _sanitize_savepoint_name( spName ); - dbi_result result = dbi_conn_queryf( writehandle, "RELEASE SAVEPOINT \"%s\";", spName ); + dbi_result result = dbi_conn_queryf( writehandle, "RELEASE SAVEPOINT \"%s\";", safeSpName ); + free( safeSpName ); if( !result ) { const char* msg; int errnum = dbi_conn_error( writehandle, &msg ); @@ -1014,8 +1020,10 @@ int rollbackSavepoint( osrfMethodContext* ctx ) { // Get the savepoint name from the method params const char* spName = jsonObjectGetString( jsonObjectGetIndex(ctx->params, spNamePos) ); + char *safeSpName = _sanitize_savepoint_name( spName ); - dbi_result result = dbi_conn_queryf( writehandle, "ROLLBACK TO SAVEPOINT \"%s\";", spName ); + dbi_result result = dbi_conn_queryf( writehandle, "ROLLBACK TO SAVEPOINT \"%s\";", safeSpName ); + free( safeSpName ); if( !result ) { const char* msg; int errnum = dbi_conn_error( writehandle, &msg ); @@ -7022,4 +7030,30 @@ static void clear_query_stack( void ) { pop_query_frame(); } +/** + @brief Remove all but safe character from savepoint name + @param sp User-supplied savepoint name + @return sanitized savepoint name, or NULL + + The caller is expected to free the returned string. Note that + this function exists only because we can't use PQescapeLiteral + without either forking libdbi or abandoning it. +*/ +static char* _sanitize_savepoint_name( const char* sp ) { + + const char* safe_chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012345789_"; + char* safeSpName = safe_malloc( strlen( sp ) + 1); + int i = 0; + int j; + char* found; + for (j = 0; j < strlen( sp ); j++) { + found = strchr(safe_chars, sp[j]); + if (found) { + safeSpName[ i++ ] = found[0]; + } + } + safeSpName[ i ] = '\0'; + return safeSpName; +} + /*@}*/ -- 2.11.0