From 9434f42c88f5cf99b3ca9980bd5a98b2bccf6a1b 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 --- 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 4fc6cbab25..c67a7a892d 100644 --- a/Open-ILS/src/c-apps/oils_sql.c +++ b/Open-ILS/src/c-apps/oils_sql.c @@ -145,6 +145,8 @@ static char* modulename = NULL; int writeAuditInfo( osrfMethodContext* ctx, const char* user_id, const char* ws_id); +static char* _sanitize_savepoint_name( const char* sp ); + /** @brief Connect to the database. @return A database connection if successful, or NULL if not. @@ -888,8 +890,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 ); @@ -959,8 +963,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 ); @@ -1030,8 +1036,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 ); @@ -7268,4 +7276,30 @@ int writeAuditInfo( osrfMethodContext* ctx, const char* user_id, const char* ws_ } } +/** + @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