From fde1b2807e9a1dddde065f0078c0c34652aabdcc Mon Sep 17 00:00:00 2001 From: Mike Rylander Date: Sat, 13 Jul 2019 14:18:09 -0400 Subject: [PATCH 1/1] LP#1836254: Handle null authtoken in PCRUD When the authtoken received from the client is the unquoted literal string "null" in a pcrud request, the drone processing the request will crash with a segmentation fault as the session verification code passes a NULL pointer to strcmp. To reproduce this bug, make the following request via srfsh: request open-ils.pcrud open-ils.pcrud.search.pgt null {"parent":null},{"flesh":-1,"flesh_fields":{"pgt":["children"]}} Note that srfsh hangs util it times out. Next, grep /var/log/syslog for the string segfault. You should find something resembling the following: Jul 12 15:29:43 buster kernel: [ 94.794920] opensrf-c[1357]: segfault at 0 ip 00007fe3bbb8b219 sp 00007fff2877a020 error 4 in liboils_pcrud.so.2.0.0[7fe3bbb82000+10000] After patching Evergreen with this commit, repeat the srfsh request again. This time, the call should return almost immediately with an osrfMethodException: "permacrud received a bad auth token: (null)." When you grep syslog for segfault this time, you should find no new occurrences. Signed-off-by: Mike Rylander Signed-off-by: Jason Stephenson --- Open-ILS/src/c-apps/oils_sql.c | 41 ++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c index da0ab62adb..42a846a7b8 100644 --- a/Open-ILS/src/c-apps/oils_sql.c +++ b/Open-ILS/src/c-apps/oils_sql.c @@ -1458,9 +1458,12 @@ static const jsonObject* verifyUserPCRUDfull( osrfMethodContext* ctx, int anon_o const char* auth = jsonObjectGetString( jsonObjectGetIndex( ctx->params, 0 ) ); jsonObject* user = NULL; + int notify_failed_verify = 1; // assume failure - // If we are /not/ in anonymous mode - if( strcmp( "ANONYMOUS", auth ) ) { + if (!auth) { + // auth token is null, fail by default + } else if( strcmp( "ANONYMOUS", auth ) ) { + // If we are /not/ in anonymous mode // See if we have the same authkey, and a user object, // locally cached from a previous call const char* cached_authkey = getAuthkey( ctx ); @@ -1478,33 +1481,37 @@ static const jsonObject* verifyUserPCRUDfull( osrfMethodContext* ctx, int anon_o jsonObjectFree( auth_object ); if( !user->classname || strcmp(user->classname, "au" )) { - - growing_buffer* msg = buffer_init( 128 ); - buffer_fadd( - msg, - "%s: permacrud received a bad auth token: %s", - modulename, - auth - ); - - char* m = buffer_release( msg ); - osrfAppSessionStatus( ctx->session, OSRF_STATUS_UNAUTHORIZED, "osrfMethodException", - ctx->request, m ); - - free( m ); jsonObjectFree( user ); user = NULL; } else if( writeAuditInfo( ctx, oilsFMGetStringConst( user, "id" ), oilsFMGetStringConst( user, "wsid" ) ) ) { // Failed to set audit information - But note that write_audit_info already set error information. + notify_failed_verify = 0; jsonObjectFree( user ); user = NULL; + } else { // succeeded + notify_failed_verify = 0; } - } else if ( anon_ok ) { // we /are/ (attempting to be) anonymous user = jsonNewObjectType(JSON_ARRAY); jsonObjectSetClass( user, "aou" ); oilsFMSetString(user, "id", "-1"); + notify_failed_verify = 0; + } + + if (notify_failed_verify) { + growing_buffer* msg = buffer_init( 128 ); + buffer_fadd( + msg, + "%s: permacrud received a bad auth token: %s", + modulename, + auth + ); + + char* m = buffer_release( msg ); + osrfAppSessionStatus( ctx->session, OSRF_STATUS_UNAUTHORIZED, "osrfMethodException", + ctx->request, m ); + free( m ); } setUserLogin( ctx, user ); -- 2.43.2