[open-ils-commits] [GIT] Evergreen ILS branch rel_2_1 updated. 2ab44b9ad4f85d6a50d271a8d6f98bf54b1432a6

Evergreen Git git at git.evergreen-ils.org
Wed Jan 16 15:05:42 EST 2013


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Evergreen ILS".

The branch, rel_2_1 has been updated
       via  2ab44b9ad4f85d6a50d271a8d6f98bf54b1432a6 (commit)
       via  120442a030e899340ebb768178d853684d3a9d21 (commit)
       via  6e02bd65f4ac6fa2f506e1e6dc842fd943f1672a (commit)
       via  c94c559a17322a464cd6f9096942ec0a8585f65c (commit)
      from  a8db04ccf3ca77d19ff1302b2661a3a2796360a7 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 2ab44b9ad4f85d6a50d271a8d6f98bf54b1432a6
Author: Galen Charlton <gmc at esilibrary.com>
Date:   Tue Jan 15 11:30:41 2013 -0500

    LP#1098377: protect against even more cstore segfaults
    
    Following up on the preceding patch, passing null
    as the savepoint name to savepoint.release and
    savepoint.rollback would also segfault cstore.
    
    Signed-off-by: Galen Charlton <gmc at esilibrary.com>
    Signed-off-by: Dan Scott <dscott at laurentian.ca>

diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c
index 033e265..d611439 100644
--- a/Open-ILS/src/c-apps/oils_sql.c
+++ b/Open-ILS/src/c-apps/oils_sql.c
@@ -953,6 +953,12 @@ int releaseSavepoint( osrfMethodContext* ctx ) {
 
 	// Get the savepoint name from the method params
 	const char* spName = jsonObjectGetString( jsonObjectGetIndex(ctx->params, spNamePos) );
+
+	if (!spName) {
+		osrfLogWarning(OSRF_LOG_MARK, "savepoint.release called with no name");
+		return -1;
+	}
+
 	char *safeSpName = _sanitize_savepoint_name( spName );
 
 	dbi_result result = dbi_conn_queryf( writehandle, "RELEASE SAVEPOINT \"%s\";", safeSpName );
@@ -1026,6 +1032,12 @@ int rollbackSavepoint( osrfMethodContext* ctx ) {
 
 	// Get the savepoint name from the method params
 	const char* spName = jsonObjectGetString( jsonObjectGetIndex(ctx->params, spNamePos) );
+
+	if (!spName) {
+		osrfLogWarning(OSRF_LOG_MARK, "savepoint.rollback called with no name");
+		return -1;
+	}
+
 	char *safeSpName = _sanitize_savepoint_name( spName );
 
 	dbi_result result = dbi_conn_queryf( writehandle, "ROLLBACK TO SAVEPOINT \"%s\";", safeSpName );

commit 120442a030e899340ebb768178d853684d3a9d21
Author: Bill Erickson <berick at esilibrary.com>
Date:   Tue Jan 15 10:58:16 2013 -0500

    Verify savepoint name is non-null
    
    Before we attempt to mangle the name, let's ensure that it's non-null.
    Otherwise, segfaults ensue.
    
    Signed-off-by: Bill Erickson <berick at esilibrary.com>
    Signed-off-by: Galen Charlton <gmc at esilibrary.com>

diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c
index 59aa5b8..033e265 100644
--- a/Open-ILS/src/c-apps/oils_sql.c
+++ b/Open-ILS/src/c-apps/oils_sql.c
@@ -874,6 +874,12 @@ int setSavepoint( osrfMethodContext* ctx ) {
 
 	// Get the savepoint name from the method params
 	const char* spName = jsonObjectGetString( jsonObjectGetIndex(ctx->params, spNamePos) );
+
+	if (!spName) {
+		osrfLogWarning(OSRF_LOG_MARK, "savepoint.set called with no name");
+		return -1;
+	}
+
 	char *safeSpName = _sanitize_savepoint_name( spName );
 
 	dbi_result result = dbi_conn_queryf( writehandle, "SAVEPOINT \"%s\";", safeSpName );

commit 6e02bd65f4ac6fa2f506e1e6dc842fd943f1672a
Author: Dan Scott <dscott at laurentian.ca>
Date:   Fri Jan 11 01:32:13 2013 -0500

    Protect against overly long savepoint names
    
    Per http://postgresql.org/docs/9.1/static/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS,
    the maximum identifier length works out to being 63 bytes (+1 for the
    null terminator), so to avoid potential memory pressure by a 10GB string
    somehow being passed in as the savepoint name, malloc no more than 64
    bytes and copy no more than 63 bytes from the incoming name to the
    escaped name.
    
    Signed-off-by: Dan Scott <dscott at laurentian.ca>
    Signed-off-by: Galen Charlton <gmc at esilibrary.com>

diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c
index c7b830c..59aa5b8 100644
--- a/Open-ILS/src/c-apps/oils_sql.c
+++ b/Open-ILS/src/c-apps/oils_sql.c
@@ -7042,11 +7042,25 @@ static void clear_query_stack( void ) {
 static char* _sanitize_savepoint_name( const char* sp ) {
 
 	const char* safe_chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012345789_";
-	char* safeSpName = safe_malloc( strlen( sp ) + 1);
+
+	// PostgreSQL uses NAMEDATALEN-1 as a max length for identifiers,
+	// and the default value of NAMEDATALEN is 64; that should be long enough
+	// for our purposes, and it's unlikely that anyone is going to recompile
+	// PostgreSQL to have a smaller value, so cap the identifier name
+	// accordingly to avoid the remote chance that someone manages to pass in a
+	// 12GB savepoint name
+	const int MAX_LITERAL_NAMELEN = 63;
+	int len = 0;
+	len = strlen( sp );
+	if (len > MAX_LITERAL_NAMELEN) {
+		len = MAX_LITERAL_NAMELEN;
+	}
+
+	char* safeSpName = safe_malloc( len + 1 );
 	int i = 0;
 	int j;
 	char* found;
-	for (j = 0; j < strlen( sp ); j++) {
+	for (j = 0; j < len; j++) {
 	found = strchr(safe_chars, sp[j]);
 		if (found) {
 			safeSpName[ i++ ] = found[0];

commit c94c559a17322a464cd6f9096942ec0a8585f65c
Author: Galen Charlton <gmc at esilibrary.com>
Date:   Fri Jan 11 02:30:50 2013 -0500

    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 at esilibrary.com>
    Signed-off-by: Dan Scott <dscott at 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
--- 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;
+}
+
 /*@}*/

-----------------------------------------------------------------------

Summary of changes:
 Open-ILS/src/c-apps/oils_sql.c |   72 ++++++++++++++++++++++++++++++++++++++--
 1 files changed, 69 insertions(+), 3 deletions(-)


hooks/post-receive
-- 
Evergreen ILS


More information about the open-ils-commits mailing list