NetBSD Problem Report #58910

From www@netbsd.org  Tue Dec 17 00:13:53 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id D1AA61A923C
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 17 Dec 2024 00:13:53 +0000 (UTC)
Message-Id: <20241217001352.65B7E1A923D@mollari.NetBSD.org>
Date: Tue, 17 Dec 2024 00:13:52 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: regcomp explodes on signedness issues
X-Send-Pr-Version: www-1.0

>Number:         58910
>Category:       lib
>Synopsis:       regcomp explodes on signedness issues
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Dec 17 00:15:00 +0000 2024
>Last-Modified:  Wed Jan 01 18:20:01 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, ...
>Organization:
The NegativeBSD Regularexpression
>Environment:
>Description:
https://mail-index.netbsd.org/tech-userlevel/2024/12/10/msg014620.html

Date: Tue, 10 Dec 2024 14:06:52 -0500
From: enh <enh@google.com>
To: "tech-userlevel@netbsd.org User-Level Technical" <tech-userlevel@netbsd.org>
Subject: regcomp() signedness issues
Message-ID: <CAJgzZooHNebuUuEYghmrAYn+uh425PSYDSgge0BvaXnCeKGd5A@mail.gmail.com>

a trivial fuzzer someone once wrote blew up on this input to regcomp()
[passed directly to regcomp() after adding a trailing '\0']:

xxd
~~/Downloads/clusterfuzz-testcase-minimized-regexec_fuzzer-5459313584832512
00000000: 6a3a 5b5d 6a3a 5b5d 6a3a 5bd9 6a3a 5b5d  j:[]j:[]j:[.j:[]

here:

==2830==ERROR: AddressSanitizer: SEGV on unknown address 0x50f020000093 (pc
0x7354670e97dd bp 0x0000ffffffd9 sp 0x7fff0d906410 T0)
==2830==The signal is caused by a WRITE memory access.
    #0 0x7354670e97dd in CHadd
bionic/libc/upstream-netbsd/lib/libc/regex/regcomp.c:1769:30
    #1 0x7354670e84be in p_b_term
bionic/libc/upstream-netbsd/lib/libc/regex/regcomp.c:1233:4
    #2 0x7354670e84be in p_bracket
bionic/libc/upstream-netbsd/lib/libc/regex/regcomp.c:1128:3
    #3 0x7354670e6492 in p_ere_exp
bionic/libc/upstream-netbsd/lib/libc/regex/regcomp.c:521:3
    #4 0x7354670e7c8b in p_re
bionic/libc/upstream-netbsd/lib/libc/regex/regcomp.c:851:19
    #5 0x7354670e5aec in regcomp_internal
bionic/libc/upstream-netbsd/lib/libc/regex/regcomp.c:379:3
    #6 0x7354670e5aec in regcomp
bionic/libc/upstream-netbsd/lib/libc/regex/regcomp.c:432:10

looking at the netbsd regex source, it seems like all accesses to `bmp`
_do_ all have appropriate `< NC` range checks, but because wint_t is
signed, the checks are wrong for negative values.

i think you want something like this patch:

diff --git a/lib/libc/regex/regcomp.c b/lib/libc/regex/regcomp.c
index 47602b77f621..2312dbaa947c 100644
--- a/lib/libc/regex/regcomp.c
+++ b/lib/libc/regex/regcomp.c
@@ -1764,8 +1764,7 @@ CHadd(struct parse *p, cset *cs, wint_t ch)
        _DIAGASSERT(p != NULL);
        _DIAGASSERT(cs != NULL);

-       assert(ch >= 0);
-       if (ch < NC)
+       if ((unsigned)ch < NC)
                cs->bmp[(unsigned)ch >> 3] |= 1 << (ch & 7);
        else {
                newwides = reallocarray(cs->wides, cs->nwides + 1,
@@ -1778,9 +1777,9 @@ CHadd(struct parse *p, cset *cs, wint_t ch)
                cs->wides[cs->nwides++] = ch;
        }
        if (cs->icase) {
-               if ((nch = towlower(ch)) < NC)
+               if ((unsigned)(nch = towlower(ch)) < NC)
                        cs->bmp[(unsigned)nch >> 3] |= 1 << (nch & 7);
-               if ((nch = towupper(ch)) < NC)
+               if ((unsigned)(nch = towupper(ch)) < NC)
                        cs->bmp[(unsigned)nch >> 3] |= 1 << (nch & 7);
        }
 }
diff --git a/lib/libc/regex/regex2.h b/lib/libc/regex/regex2.h
index fbfff0daf0f8..ee37044defc9 100644
--- a/lib/libc/regex/regex2.h
+++ b/lib/libc/regex/regex2.h
@@ -135,8 +135,7 @@ CHIN1(cset *cs, wint_t ch)
 {
        unsigned int i;

-       assert(ch >= 0);
-       if (ch < NC)
+       if ((unsigned)ch < NC)
                return (((cs->bmp[(unsigned)ch >> 3] & (1 << (ch & 7))) !=
0) ^
                    cs->invert);
        for (i = 0; i < cs->nwides; i++) {
@@ -160,8 +159,7 @@ static __inline int
 CHIN(cset *cs, wint_t ch)
 {

-       assert(ch >= 0);
-       if (ch < NC)
+       if ((unsigned)ch < NC)
                return (((cs->bmp[(unsigned)ch >> 3] & (1 << (ch & 7))) !=
0) ^
                    cs->invert);
        else if (cs->icase)

you can also see i've also removed the assert()s since i don't think
anyone's actually building this code with them enabled, and the false sense
of security from reading that assert is quite likely what caused that this
bug to be introduced in the first place...

>How-To-Repeat:
example given
>Fix:
1. Add ATF tests to exercise this case and any other similar cases.
2. Apply the patch given by enh.
3. Review the code for other bugs of this class and make sure it handles all possible inputs gracefully.

>Audit-Trail:
From: RVP <rvp@SDF.ORG>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: lib/58910: regcomp explodes on signedness issues
Date: Mon, 30 Dec 2024 07:07:40 +0000 (UTC)

 See also PR bin/58092 which seems related.

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58910 CVS commit: src/tests/lib/libc/regex
Date: Wed, 1 Jan 2025 13:13:49 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Wed Jan  1 18:13:48 UTC 2025

 Modified Files:
 	src/tests/lib/libc/regex: Makefile
 Added Files:
 	src/tests/lib/libc/regex: t_regex_binary.c

 Log Message:
 Add a test for PR/58910


 To generate a diff of this commit:
 cvs rdiff -u -r1.11 -r1.12 src/tests/lib/libc/regex/Makefile
 cvs rdiff -u -r0 -r1.1 src/tests/lib/libc/regex/t_regex_binary.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58910 CVS commit: src/lib/libc/regex
Date: Wed, 1 Jan 2025 13:19:50 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Wed Jan  1 18:19:50 UTC 2025

 Modified Files:
 	src/lib/libc/regex: regcomp.c regex2.h

 Log Message:
 PR/58910: enh at google dot com: Fix signed character issue in character
 ranges.


 To generate a diff of this commit:
 cvs rdiff -u -r1.48 -r1.49 src/lib/libc/regex/regcomp.c
 cvs rdiff -u -r1.15 -r1.16 src/lib/libc/regex/regex2.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search