From 9490d34c2db381484987df7fe428c31c7215121b Mon Sep 17 00:00:00 2001 From: Mike Rylander Date: Fri, 5 Feb 2021 09:03:40 -0500 Subject: [PATCH] LP#1847805: Protect cursor name from recursive calls In pcrud mode we were not being careful enough about when the cursor name was created, and permission verification could clear the cursor created in the caller. This had the effect of disabling app-level limit and offset calculation. Signed-off-by: Mike Rylander --- Open-ILS/src/c-apps/oils_sql.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c index 24072c6128..ff16763c20 100644 --- a/Open-ILS/src/c-apps/oils_sql.c +++ b/Open-ILS/src/c-apps/oils_sql.c @@ -5969,9 +5969,14 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ int flesh_depth = 0; int offset = 0; int limit = 0; + int can_get_cursor = 1; + int do_cursor = 0; int cursor_page_size = 50; - if (*methodtype != 'r') cursor_name = random_cursor_name(); /* Use a cursor for all but .retrieve */ + if (cursor_name) can_get_cursor = 0; // There is already a cursor, and it is not local + if (*methodtype != 'r' && can_get_cursor) cursor_name = random_cursor_name(); /* Use a cursor for all but .retrieve */ + + if (cursor_name && can_get_cursor) do_cursor = 1; char* sql = buildSELECT( where_hash, query_hash, class_meta, ctx ); if( !sql ) { @@ -6019,7 +6024,7 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ } - if (cursor_name) { + if (do_cursor) { const jsonObject* limit_o = jsonObjectGetKeyConst( query_hash, "limit" ); if( limit_o ) { limit = (int) jsonObjectGetNumber( limit_o ); @@ -6114,14 +6119,14 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ rs_size = (int *) safe_malloc( sizeof(int) ); // will be freed by sessionDataFree() unsigned long long result_count = cursor_page_size; - if (!cursor_name) { + if (!do_cursor) { result_count = dbi_result_get_numrows( result ); } *rs_size = (int) result_count * (flesh_depth + 1); // yes, we could lose some bits, but come on osrfHashSet( (osrfHash *) ctx->session->userData, rs_size, "rs_size_req_%d", ctx->request ); } - if (cursor_name) { + if (do_cursor) { dbi_result_free( result ); // toss the declare result result = dbi_conn_queryf( dbhandle, "FETCH %d FROM \"%s\";", cursor_page_size, cursor_name ); @@ -6166,7 +6171,7 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ } else { if( !enforce_pcrud || !need_to_verify || verifyObjectPCRUD( ctx, class_meta, row_obj, 0 /* means check user data for rs_size */ )) { - if (cursor_name) { + if (do_cursor) { if (offset > -1 && offset) { offset--; } else { @@ -6180,7 +6185,7 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ } } } - if (cursor_name && !cursor_page_remaining) { // fetch next page + if (do_cursor && !cursor_page_remaining) { // fetch next page cursor_page_remaining = cursor_page_size; dbi_result_free( result ); result = dbi_conn_queryf( dbhandle, "FETCH %d FROM \"%s\";", cursor_page_size, cursor_name ); @@ -6207,8 +6212,8 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ } } } while( - (!cursor_name && dbi_result_next_row(result)) || ( - cursor_name && limit && ( // limit is -1 (none) or not yet decremented to 0 + (!do_cursor && dbi_result_next_row(result)) || ( + do_cursor && limit && ( // limit is -1 (none) or not yet decremented to 0 (cursor_page_remaining == cursor_page_size && dbi_result_first_row(result)) || // on a new page, and there's a row (cursor_page_remaining != cursor_page_size && dbi_result_next_row(result)) // on the same page, and there's a row ) @@ -6223,7 +6228,7 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ /* clean up the query */ dbi_result_free( result ); - if (cursor_name) { + if (do_cursor) { result = dbi_conn_queryf( dbhandle, "CLOSE \"%s\";", cursor_name ); dbi_result_free( result ); free_cursor_name(); // release the cursor name now so recursive fleshing calls can make their own without leaking -- 2.11.0