svn commit: r278739 - head/lib/libc/regex

[Xin Li]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 02/13/15 16:37, Bryan Drewery wrote:
> On 2/13/2015 6:23 PM, Xin LI wrote:
>> Author: delphij Date: Sat Feb 14 00:23:53 2015 New Revision:
>> 278739 URL: https://svnweb.freebsd.org/changeset/base/278739
>> 
>> Log: Disallow pattern spaces which would cause intermediate
>> calculations to overflow size_t.
>> 
>> Obtained from:	DragonFly
>> (2841837793bd095a82f477e9c370cfe6cfb3862c dillon) Security:	CERT
>> VU#695940 MFC after:	3 days
>> 
>> Modified: head/lib/libc/regex/regcomp.c
>> 
>> Modified: head/lib/libc/regex/regcomp.c 
>> ==============================================================================
>>
>> 
- --- head/lib/libc/regex/regcomp.c	Sat Feb 14 00:03:43 2015	(r278738)
>> +++ head/lib/libc/regex/regcomp.c	Sat Feb 14 00:23:53 2015
>> (r278739) @@ -192,6 +192,7 @@ regcomp(regex_t * __restrict preg, 
>> struct parse *p = &pa; int i; size_t len; +	size_t maxlen; #ifdef
>> REDEBUG #	define	GOODFLAGS(f)	(f) #else @@ -213,7 +214,23 @@
>> regcomp(regex_t * __restrict preg, g = (struct re_guts
>> *)malloc(sizeof(struct re_guts)); if (g == NULL) 
>> return(REG_ESPACE); +	/* +	 * Limit the pattern space to avoid a
>> 32-bit overflow on buffer +	 * extension.  Also avoid any signed
>> overflow in case of conversion +	 * so make the real limit based
>> on a 31-bit overflow. +	 * +	 * Likely not applicable on 64-bit
>> systems but handle the case +	 * generically (who are we to stop
>> people from using ~715MB+ +	 * patterns?). +	 */ +	maxlen =
>> ((size_t)-1 >> 1) / sizeof(sop) * 2 / 3; +	if (len >= maxlen) { +
>> free((char *)g);
> 
> I was planning to submit a patch for review to remove all of this 
> casting / and discuss.
> 
> In this example the malloc is casted to struct re_gets* but the
> free is casted to char *. Why different and why cast in free at
> all?

No, I don't think there is legitimate reason to do casts in these
places (both malloc and free's), these casts are not functional, and
should be eliminated per style(9).  In DragonFly, I think the cast was
done mainly to match what's done in the rest of regex code, which is
the traditional BSD way of handling third party code.

If, however, you would like to remove them altogether, please feel
free to do so once it's considered ready to go, as regex(3) is mostly
unmaintained nowadays, and there is no upstream anymore.

PS: Gabor have proposed to replace Spencer regex(3) with TRE but that
was never finished, which would require some work (probably a good SoC
project?).  My $0.02 recommendation is to leave the code as-is for now.

Cheers,
- -- 
Xin LI <delphij at delphij.net>    https://www.delphij.net/
FreeBSD - The Power to Serve!           Live free or die
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.1.1 (FreeBSD)

iQIcBAEBCgAGBQJU3p8IAAoJEJW2GBstM+nsF7gP/j/5V5BsPObh1aOSkM4fSsd+
l68ZuBw+YbCcbFs7EMjpFx7prqL+KyZ29Cq5blpROVMz94cV6jmqVE8O2D1WXNbk
V6E4DxX//T2SfHjLnTPKBGn3Pdx/Gi3sLpkzGhAwqhPOVXCS56sOFt2fuAMfC7oq
k9OOLUHjtcocEG7p/xk0fRhM9GLylnIdpZqGVugb+QmxI9coo6DgsFXddKSnLzwe
D15j73sxQ9zgfWu3LThIFm1oE9MvbllUn/kiilZNPrfKljOoANiz2NwbY2kNBeM5
jjM1YpYKsqGuLEZx0UA0GnAJe7jW5yrMGxCTImLd/K9AWg48YzaB+76oQ6JQgSWh
6prDpse7BNdzSeTBf8VCX0fyLW7XWHDqCl3f2L3ZL/mF6zydVFfsKCfgYiHeUJzl
3e5GlVSyszz/fK0QPcau7KVO+FT/1dmbMBRDFhlC4ziITLvPe2ycxE15DvrhcVGM
ifVTTEpH0tUv/e3Ac9lW9siENdssVfx9kx/pY5mfVU3bTntYCK8bGM7Nk7ayJQz5
A2C8oJP1bE41mqjGDihi0+aiZHmuW6WvPeRwM0okGuH7Lma4gQZrzs8hN826evY1
DueeYGrinqx5T8uN/HEQRkgFfyE/n5YDJvOwOmKQlOlz+m0hF7nX3kSTwUMZAmEZ
o8rTmSN9k2e5Mu/0Ejd5
=8I1d
-----END PGP SIGNATURE-----