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 <gmc@esilibrary.com>
Signed-off-by: Dan Scott <dscott@laurentian.ca>
Conflicts:
	Open-ILS/src/c-apps/oils_sql.c

diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c
index 2f19ddb..c7b830c 100644 (file)
--- 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;
+}
+
 /*@}*/