[PATCH v1] xstrtol: 1 is not a valid base

--qyofym5wuv3nffow
Content-Type: text/plain; protected-headers=v1; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
From: Alejandro Colomar <[email protected]>
To: [email protected]
Cc: Alejandro Colomar <[email protected]>, Paul Eggert <[email protected]>, 
	=?utf-8?B?xJBvw6BuIFRy4bqnbiBDw7RuZw==?= Danh <[email protected]>, Eli Schwartz <[email protected]>, Sam James <[email protected]>, 
	Serge Hallyn <[email protected]>, Iker Pedrosa <[email protected]>, 
	"Andrew J. Hesford" <[email protected]>, Michael Vetter <[email protected]>, [email protected]
Subject: [PATCH v1] xstrtol: 1 is not a valid base
MIME-Version: 1.0

If xstrtol() was being called with a base of 1, under some conditions it
would invoke Undefined Behavior.

Here's the code that would trigger UB:

	char  *end;

	xstrtol(str, &end, 1, ...);  // Let's ignore trailing args.

The reason why this triggers UB is that since the following line lets a
base of 1 go through:

	assure (0 <=3D strtol_base && strtol_base <=3D 36);

then we arrive at this call:

	tmp =3D __strtol (s, p, strtol_base);

which sets errno to EINVAL and returns 0 immediately, without updating
the 'p' pointer.  Then, the following line of code:

	if (*p =3D=3D s)

dereferences an uninitialized pointer.

This was found while searching for examples of why strtol(3) is a bad
API, and how it makes it so easy to misuse.

Fixes: 034a18049cbc (2014-12-20, "assure: new module")
Link: <https://github.com/void-linux/void-packages/issues/51261#issuecommen=
t-2237013621>
Cc: Paul Eggert <[email protected]>
Cc: =C4=90o=C3=A0n Tr=E1=BA=A7n C=C3=B4ng Danh <[email protected]>
Cc: Eli Schwartz <[email protected]>
Cc: Sam James <[email protected]>
Cc: Serge Hallyn <[email protected]>
Cc: Iker Pedrosa <[email protected]>
Cc: "Andrew J. Hesford" <[email protected]>
Cc: Michael Vetter <[email protected]>
Cc: <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
---
Range-diff against v0:
-:  ---------- > 1:  49c4c25b0a xstrtol: 1 is not a valid base

 lib/xstrtol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/xstrtol.c b/lib/xstrtol.c
index e4bce43681..575c16d45f 100644
--- a/lib/xstrtol.c
+++ b/lib/xstrtol.c
@@ -83,7 +83,7 @@ __xstrtol (const char *s, char **ptr, int strtol_base,
   __strtol_t tmp;
   strtol_error err =3D LONGINT_OK;
=20
-  assure (0 <=3D strtol_base && strtol_base <=3D 36);
+  assure (0 =3D=3D strtol_base || (2 <=3D strtol_base && strtol_base <=3D =
36));
=20
   p =3D (ptr ? ptr : &t_ptr);
=20
--=20
2.45.2


--qyofym5wuv3nffow
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmaZSDcACgkQnowa+77/
2zK+Qw//Ysx/pgeInyaB6ZApS7kIoEhblVOSYwhuLeJ8Mo/SwUxqWZuh6iMXhoeP
9d3Nogr9Wvmv2K4k6Zmo8vGLqv+y7sFzgcyGdH9MW98LXEErA9ywiIozqdeEoM+p
2ErAmt+ZoviTPXF3ALddFho4bz9zq1FEIlUYR8Hq3J7FkiyLUhOAGFpv0WpnlGaQ
bmkOUhR1pf76x4SLFnGccVK4z4vF/40k4eqlMN/Wv3rQR/6RvbGpMB06/6yV8PeO
gyKMp2lENaOP+3guLpL6WqIQTv5ZYNFTiECEpN8Tm0jTIT3TlxmiN7vG+ChxQL1E
ql40syIbrOLgUJpvxsYn+7FuSk7lfwGgrMEX2iGlIsIQg4b2n1VdXHcMxvLMajpS
KycHDudTcUbfIeV1WfN2i8qggmhchXL3GZIcI63gGdGmKrVBWyOYWkDg36B0eUUK
3KO3zjmwks6Me+NIOw2m45d4gX2mRGgYUfrrly4JhMWmKv/xTSEWH7xMZFqEBcvv
wk7keJ4lSpxJ3mswiXthR6qDs1YQ4w6VSq1DUB5YjenIN1EcZ/rDxqjUHkFxnYDR
X621KjbKdEB6bcqQML2tAWjmtJSkNB948QSL/oxYOP5alAAUX9Y1rOEtOv4i3NZh
+QyTb7jfrRL5yc0jppPiWoMFDcPiMdEKoNRN8ZG7incZmHq42z4=
=GsFC
-----END PGP SIGNATURE-----

--qyofym5wuv3nffow--