Skip to content

Fix SQL injection via message ID in DbTrackingModule.persist - #567

Merged
uhurusurfa merged 3 commits into
OpenAS2:masterfrom
JakeHuneau:fix/db-tracking-sql-injection
Jul 25, 2026
Merged

Fix SQL injection via message ID in DbTrackingModule.persist#567
uhurusurfa merged 3 commits into
OpenAS2:masterfrom
JakeHuneau:fix/db-tracking-sql-injection

Conversation

@JakeHuneau

Copy link
Copy Markdown
Contributor

The existence-check SELECT concatenated the message ID directly into the SQL, and for inbound messages that value is the partner-controlled Message-ID header. A crafted header could inject SQL (e.g. "x' OR '1'='1" made a new message match an existing row, corrupting tracking) or break the query outright, and persist() swallows the resulting exception so the tracking record was silently lost.

Parameterise the existence-check SELECT and escape the message ID in the UPDATE WHERE clause using the existing formatField() helper (the same escaping the SET values already use). Adds a test proving crafted IDs are stored/matched as opaque literals rather than interpreted as SQL.

The existence-check SELECT concatenated the message ID directly into the SQL,
and for inbound messages that value is the partner-controlled Message-ID header.
A crafted header could inject SQL (e.g. "x' OR '1'='1" made a new message match
an existing row, corrupting tracking) or break the query outright, and persist()
swallows the resulting exception so the tracking record was silently lost.

Parameterise the existence-check SELECT and escape the message ID in the UPDATE
WHERE clause using the existing formatField() helper (the same escaping the SET
values already use). Adds a test proving crafted IDs are stored/matched as opaque
literals rather than interpreted as SQL.

@uhurusurfa uhurusurfa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@uhurusurfa
uhurusurfa merged commit 6362316 into OpenAS2:master Jul 25, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants