chore(mod_alworkshops): cast hidden_id to int for consistency #10

Closed
imre.agent wants to merge 1 commits from fix/alworkshops-hidden-id-sqli into main
Owner

Correction — this is hardening, not a security fix

An earlier analysis flagged hidden_id as a SQL-injection vector via lines 207/239. A follow-up control-flow review (and this PR's own review) found that to be a false positive. Keeping the record straight:

  • Line 190 ($app['user_id'] = $_POST['hidden_id']) lives in the event/webinar branch (the if at 106, taken only when usr+email are set). There the tainted value reaches only insert() (193), and MySqlHelper::insert() escapes every value with mysqli_real_escape_string (helper line 177).
  • The raw-concatenated queries at 207/239 are in the mutually-exclusive workshop else if (206, taken only when usr+email are not set). On that path line 190 never ran, so $app['user_id'] still holds the (int)-cast value from the array init at line 89 — not tainted.

So there is no exploitable injection through hidden_id.

What remains

The one-line change stands on its own merit as consistency / defense-in-depth, not as a vulnerability fix:

cadline/modules/modules/mod_alworkshops/assets/ajax/alworkshops.class.php:190
-    $app['user_id'] = $_POST['hidden_id'];
+    $app['user_id'] = (int)$_POST['hidden_id'];

user_id is numeric and is already (int)-cast everywhere else in this same file (e.g. line 89). The cast is transparent for legitimate input and keeps the field's type consistent should the value ever flow somewhere unescaped after a future refactor.

Harmless to merge, harmless to close — no live vulnerability rides on it. Flagging the rationale correction explicitly so the change is not misread as patching a real SQLi.

## Correction — this is hardening, not a security fix An earlier analysis flagged `hidden_id` as a SQL-injection vector via lines 207/239. **A follow-up control-flow review (and this PR's own review) found that to be a false positive.** Keeping the record straight: - Line 190 (`$app['user_id'] = $_POST['hidden_id']`) lives in the **event/webinar branch** (the `if` at 106, taken only when `usr`+`email` are set). There the tainted value reaches **only** `insert()` (193), and `MySqlHelper::insert()` escapes every value with `mysqli_real_escape_string` (helper line 177). - The raw-concatenated queries at 207/239 are in the **mutually-exclusive workshop `else if`** (206, taken only when `usr`+`email` are *not* set). On that path line 190 never ran, so `$app['user_id']` still holds the `(int)`-cast value from the array init at line 89 — not tainted. So there is **no exploitable injection** through `hidden_id`. ## What remains The one-line change stands on its own merit as **consistency / defense-in-depth**, not as a vulnerability fix: ``` cadline/modules/modules/mod_alworkshops/assets/ajax/alworkshops.class.php:190 - $app['user_id'] = $_POST['hidden_id']; + $app['user_id'] = (int)$_POST['hidden_id']; ``` `user_id` is numeric and is already `(int)`-cast everywhere else in this same file (e.g. line 89). The cast is transparent for legitimate input and keeps the field's type consistent should the value ever flow somewhere unescaped after a future refactor. Harmless to merge, harmless to close — no live vulnerability rides on it. Flagging the rationale correction explicitly so the change is not misread as patching a real SQLi.
imre.agent added 1 commit 2026-07-22 11:03:17 +02:00
The workshop-application handler assigned $_POST['hidden_id'] straight into $app['user_id'], which is then concatenated unescaped into two SELECT statements (user_id=" . $app['user_id']). An attacker could inject SQL through the hidden_id POST field.

user_id is a numeric key everywhere else in this same file (e.g. line 89 already uses (int)$_POST['user_id']); applying the same (int) cast closes the injection with no behaviour change.

Assisted-by: claude-code@claude-opus-4-8
imre.agent changed title from fix(mod_alworkshops): cast hidden_id to int to close SQL injection to chore(mod_alworkshops): cast hidden_id to int for consistency 2026-07-22 17:29:38 +02:00
imre.agent closed this pull request 2026-07-23 06:46:31 +02:00

Pull request closed

Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cadline_web/www_archline_hu#10
No description provided.