GHSA-rw77-vq4g-x3hp: SQL Injection

Summary

The StopWords::add() method in phpMyFAQ builds a SQL INSERT statement using sprintf() and inserts the user-supplied stop word value directly into the query string without calling the application's database escaping function on it. A sibling method, StopWords::update(), which modifies an existing stop word, correctly escapes the same kind of input. The omission is isolated to the add() (insert) code path.

An authenticated administrator who can reach the stop-word management feature can submit a crafted value as the "word" parameter that breaks out of the SQL string literal and injects arbitrary SQL, including statements to drop tables, exfiltrate data, or modify other rows in the database.

---

Affected Code

File: phpmyfaq/src/phpMyFAQ/StopWords.php Method: add() (approx. lines 60–75 in the audited revision)

php $sql = sprintf( "INSERT INTO %s VALUES(%d, '%s', '%s')", $this->getTableName(), $id, $this->configuration->getDb()->escape($this->language), // language IS escaped $word // <-- $word is NOT escaped );

$word is taken directly from the administrative form input (the new stop word to add) and concatenated into the SQL string via sprintf("'%s'", ...) with no call to the database driver's escape() method.

Contrast with the safe sibling method

Method: update() (line 82 in the audited revision)

php $this->configuration->getDb()->escape($word)

update() — which modifies an existing stop word — correctly escapes $word before use. add() does not perform the same escaping on the equivalent value. This inconsistency between two methods handling the same data type is the root cause: the escaping convention used throughout the rest of the file was not applied uniformly to this one insertion path.

---

Proof of Concept

Precondition: Attacker has valid administrator credentials (or has otherwise obtained an authenticated administrator session, e.g. via a separate session-hijacking or CSRF vector).

Attack steps:

1. Authenticate to the phpMyFAQ administration panel. 2. Navigate to the Stop Words management feature. 3. Submit a new stop word with the following value instead of a normal word:

test', 'en'); DROP TABLE faqstopwords; --

4. The resulting SQL statement sent to the database becomes (table/column names approximate, based on the traced sprintf template):

sql INSERT INTO faqstopwords VALUES(1, 'en', 'test', 'en'); DROP TABLE faqstopwords; --')

5. The injected DROP TABLE faqstopwords; statement executes as a second SQL statement (subject to the database driver/PDO configuration permitting multi-statement execution; even where multi-statement execution is disabled, the same injection point allows classic single-statement SQLi techniques such as UNION-based data extraction or boolean/time-based blind injection against other tables the database user can access).

---

Impact

- Confidentiality: An attacker with this access can use UNION-based or blind SQL injection techniques to read data from other tables in the database (e.g. user credentials, FAQ content marked as private/internal, session data) that the database user account has permission to access. - Integrity: Arbitrary INSERT/UPDATE/DELETE statements can be appended, allowing modification of unrelated application data. - Availability: As demonstrated in the PoC, structural statements like DROP TABLE can be injected, directly impacting application availability.

Mitigating factor: Exploitation requires an authenticated administrator session. This is not exploitable by an anonymous or low-privilege user. This lowers the severity from Critical/High to Medium, consistent with phpMyFAQ's own threat model where administrators are a trusted role — but it remains a genuine defense-in-depth failure: a compromised or malicious admin account (or an admin tricked via a separate vector such as CSRF, if no CSRF protection exists on this specific form) can leverage this into full database compromise, which a properly parameterized query would have prevented even in that scenario.

---

Root Cause

The codebase's established pattern for this class (StopWords.php) is to escape all string values via $this->configuration->getDb()->escape($value) before placing them into a sprintf()-built SQL string. This pattern is correctly applied to:

- $this->language in add() - $word in update()

It is not applied to $word in add(). This is a single-line omission, not a structural design flaw — the safe pattern already exists in the same file and the same class, just inconsistently applied across the two methods that handle the same input type.

---

Recommended Fix

Apply the same escaping already used in update() and already used for $this->language in the same add() method:

php $sql = sprintf( "INSERT INTO %s VALUES(%d, '%s', '%s')", $this->getTableName(), $id, $this->configuration->getDb()->escape($this->language), $this->configuration->getDb()->escape($word) // FIX: escape $word here );

Stronger recommended fix (defense in depth): Migrate this query, and ideally all sprintf()-built SQL in this class, to parameterized/prepared statements (e.g. PDO::prepare() with bound parameters) rather than string-escaping plus sprintf(). Escaping is correct when applied consistently, but prepared statements remove this entire vulnerability class structurally and prevent any future omission of this kind from being exploitable.