Skip to content

GHSA-623j-hfpc-mrc4

CVE Information

SUMMARY

preparse() rewrites SQL placeholders into ':pN' numbered form, using an output buffer sized as strlen(statement)*7+16 , a budget of at most 7 output bytes per source byte, based on the largest placeholder number ever being ':p99999' (7 characters). This exact invariant was the subject of CVE-2026-10879 and its follow-up fix CVE-2026-14739 earlier this year, both of which constrain the placeholder counter (idx) through its normal '?' increment path.

There's a second, unguarded way idx gets set that neither fix touches: the ':1'-style numeric placeholder branch parses the digits with atoi() and assigns the result straight to idx, with no range check at all:

else if (isDIGIT(*src)) { /* :1 */
    const int pln = atoi(src);
    if (PS_return(DBIpp_ph_cn)) {
        idx = pln; // no validation
        ...

A placeholder number that overflows a C int e.g. ":2147483648", ten ordinary digits, nothing suspicious-looking in real SQL, makes atoi() return -2147483648 on glibc (I tested this directly). That poisons idx. Every '?' mark that follows then bypasses the idx >= 99999 guard added by the CVE-2026-14739 fix (a large negative number always satisfies < 99999) and sprintf(start,":p%d", idx++) writes up to 13 characters instead of the 7 the buffer budgets for — a real, accumulating heap buffer overflow write.

I confirmed this isn't just theoretical: I extracted the real allocation formula and both real branches from DBI.xs verbatim into a small harness (only newSV()/SvPVX() swapped for an equivalent-size malloc(), no logic changed) and compiled under AddressSanitizer. Clean heap-buffer-overflow WRITE of 14 bytes past a 233-byte buffer, pointing directly at the real sprintf() sink line, triggered by ":2147483648" followed by 20 '?' marks. I also built a control test with a pln < 1 || pln >= 99999 check added — it cleanly rejects the malicious input with no overflow, and I confirmed the same check doesn't break legitimate mixed :1/? statements.

I verified this is still present and unguarded on current HEAD (commit 4a8502a, past the 1.651 release), and confirmed neither CVE-2026-10879's nor CVE-2026-14739's fix diff touches this atoi()/':1' branch at all, so I believe this is a genuinely distinct issue rather than a restatement of either.

SUGGESTED FIX

Validate pln against the same [1, 99999) bound the '?' path already enforces, before trusting it into idx:

const int pln = atoi(src);
if (PS_return(DBIpp_ph_cn)) {
    if (pln < 1 || pln >= 99999) {
        // reject, same pattern as the existing idx>=99999 error path
    }
    idx = pln;
    ...

Possibly also worth switching atoi() to strtol() with an explicit ERANGE check at the parse site itself, so this doesn't depend on atoi()'s implementation-defined overflow behavior at all, given this function has needed more than one pass already, closing off the whole class in one go might be worth the extra line.