[PATCH] apr_dbd_sqlite3: prevent OOB read in blob binding (affects 1.6.3 and trunk)

--000000000000f07fc2065631b608
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hello,

I discovered a security issue in the SQLite3 DBD driver of apr-util
(version 1.6.3, and it also exists in the current trunk).

The function dbd_sqlite3_bind() in apr_dbd_sqlite3.c reads a blob size
via atoi() from a user-supplied string and passes it directly to
sqlite3_bind_blob() without any validation against the actual length
of the data buffer.

An attacker can supply a size larger than the real data length,
causing an out=E2=80=91of=E2=80=91bounds read when SQLite copies the blob. =
This may
lead to information disclosure or a crash (DoS).

The issue affects the text=E2=80=91based BLOB binding path (pquery/pselect
family). Binary BLOB binding (pbquery/pbselect) is not affected by
this specific code path, but it also lacks validation (a separate
issue).

The proposed patch adds a safety check: if the announced size exceeds
strlen(data), the parameter is bound as NULL instead of a blob. This
prevents the OOB read and falls back to a safe behavior.

This change assumes that text=E2=80=91based BLOB bindings are only used for
textual data (the common case). For genuine binary data, callers
should migrate to the binary bind functions (pbquery/pbselect), which
already expect a binary size argument.

The patch follows the Apache C Language Style Guide (4=E2=80=91space indent=
,
no tabs, brace placement, spacing around operators).

Please review and consider applying.

Thank you,
Max Lane
<[email protected]>

---
Index: dbd/apr_dbd_sqlite3.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- dbd/apr_dbd_sqlite3.c (working copy)
+++ dbd/apr_dbd_sqlite3.c (working copy)
@@ -458,7 +458,22 @@
                 char *data =3D (char *)values[j];
                 int size =3D atoi((char*)values[++j]);

                 /* skip table and column */
                 j +=3D 2;

-                sqlite3_bind_blob(stmt, i + 1, data, size, SQLITE_STATIC);
+                /*
+                 * Validate that the announced size does not exceed the
actual
+                 * length of the data. Otherwise an attacker could cause a=
n
+                 * out-of-bounds read in sqlite3_bind_blob().
+                 *
+                 * For binary BLOB data the binary bind functions
+                 * (pbquery/pbselect) should be used instead.
+                 */
+                if (size > (int)strlen(data)) {
+                    sqlite3_bind_null(stmt, i + 1);
+                }
+                else {
+                    sqlite3_bind_blob(stmt, i + 1, data, size,
SQLITE_STATIC);
+                }
                 }
                 break;

--000000000000f07fc2065631b608
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hello,<br><br>I discovered a security issue in the SQLite3=
 DBD driver of apr-util<br>(version 1.6.3, and it also exists in the curren=
t trunk).<br><br>The function dbd_sqlite3_bind() in apr_dbd_sqlite3.c reads=
 a blob size<br>via atoi() from a user-supplied string and passes it direct=
ly to<br>sqlite3_bind_blob() without any validation against the actual leng=
th<br>of the data buffer.<br><br>An attacker can supply a size larger than =
the real data length,<br>causing an out=E2=80=91of=E2=80=91bounds read when=
 SQLite copies the blob. This may<br>lead to information disclosure or a cr=
ash (DoS).<br><br>The issue affects the text=E2=80=91based BLOB binding pat=
h (pquery/pselect<br>family). Binary BLOB binding (pbquery/pbselect) is not=
 affected by<br>this specific code path, but it also lacks validation (a se=
parate<br>issue).<br><br>The proposed patch adds a safety check: if the ann=
ounced size exceeds<br>strlen(data), the parameter is bound as NULL instead=
 of a blob. This<br>prevents the OOB read and falls back to a safe behavior=
.<br><br>This change assumes that text=E2=80=91based BLOB bindings are only=
 used for<br>textual data (the common case). For genuine binary data, calle=
rs<br>should migrate to the binary bind functions (pbquery/pbselect), which=
<br>already expect a binary size argument.<br><br>The patch follows the Apa=
che C Language Style Guide (4=E2=80=91space indent,<br>no tabs, brace place=
ment, spacing around operators).<br><br>Please review and consider applying=
.<br><br>Thank you,<br>Max Lane<br>&lt;<a href=3D"mailto:max.lane.dev@gmail=
.com">[email protected]</a>&gt;<br><br>---<br>Index: dbd/apr_dbd_sqlit=
e3.c<br>=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D<br>--- dbd/apr=
_dbd_sqlite3.c	(working copy)<br>+++ dbd/apr_dbd_sqlite3.c	(working copy)<b=
r>@@ -458,7 +458,22 @@<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0char *data =3D (char *)values[j];<br>=C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int size =3D atoi((char*)values[++j])=
;<br>=C2=A0<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0/* skip table and column */<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
 =C2=A0 =C2=A0 =C2=A0j +=3D 2;<br>=C2=A0<br>- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3_bind_blob(stmt, i + 1, data, size, SQLIT=
E_STATIC);<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/*<b=
r>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * Validate that=
 the announced size does not exceed the actual<br>+ =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * length of the data. Otherwise an attac=
ker could cause an<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 * out-of-bounds read in sqlite3_bind_blob().<br>+ =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 * For binary BLOB data the binary bind function=
s<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * (pbquery/p=
bselect) should be used instead.<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 */<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0if (size &gt; (int)strlen(data)) {<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3_bind_null(stmt, i + 1)=
;<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br>+ =C2=A0=
 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else {<br>+ =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3_bind_blob(st=
mt, i + 1, data, size, SQLITE_STATIC);<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0}<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0}<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0break;</div>

--000000000000f07fc2065631b608--