-
Notifications
You must be signed in to change notification settings - Fork 1
π‘οΈ Sentinel: [HIGH] Fix authorization bypass in database row mutation APIs #483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| ## 2025-04-26 - Missing Table Existence Check in Row Mutations | ||
| **Vulnerability:** Authorization bypass. PUT (update) and DELETE endpoints in /api/database/tables/:table/rows did not verify that the target table was a valid user table (using assertTableExists), unlike GET and POST endpoints. | ||
| **Learning:** This oversight allowed potential modification/deletion of arbitrary or internal tables if an attacker guessed the table name, bypassing the safety mechanisms intended to restrict queries to user-facing base tables. | ||
| **Prevention:** Always apply the exact same authorization and existence checks across all CRUD operations for a given resource. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -970,6 +970,11 @@ async function handleUpdateRow( | |
| return; | ||
| } | ||
|
|
||
| if (!(await assertTableExists(runtime, tableName))) { | ||
| sendJsonError(res, `Table "${tableName}" not found`, 404); | ||
| return; | ||
| } | ||
|
|
||
| const setClauses = Object.entries(body.data).map(([col, val]) => | ||
| sqlAssign(col, val), | ||
| ); | ||
|
Comment on lines
970
to
980
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security: Risk of SQL Injection in Update HandlerThe code constructs SQL Recommendation: Example (conceptual): await db.execute('UPDATE table SET col1 = $1 WHERE col2 = $2', [val1, val2]); |
||
|
|
@@ -1016,6 +1021,11 @@ async function handleDeleteRow( | |
| return; | ||
| } | ||
|
|
||
| if (!(await assertTableExists(runtime, tableName))) { | ||
| sendJsonError(res, `Table "${tableName}" not found`, 404); | ||
| return; | ||
| } | ||
|
Comment on lines
+1024
to
+1027
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to |
||
|
|
||
| const whereClauses = Object.entries(body.where).map(([col, val]) => | ||
| sqlPredicate(col, val), | ||
| ); | ||
|
Comment on lines
1021
to
1031
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security: Risk of SQL Injection in Delete HandlerThe construction of the Recommendation: |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While adding the
assertTableExistscheck correctly addresses the authorization bypass, its placement afterreadJsonBodyis less than optimal. Moving this check to the very beginning of the function (before parsing the request body) would follow the 'fail-fast' principle, avoiding unnecessary resource consumption (CPU/memory for JSON parsing) when the target table does not exist. This would also align with the implementation inhandleGetRows.