tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: [GitHub] [tomcat] alpire opened a new pull request #176: CoyoteAdapter: fix out-of-bounds read in checkNormalize
Date Mon, 01 Jul 2019 15:06:55 GMT
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Mark,

On 6/29/19 16:01, Mark Thomas wrote:
> On 29/06/2019 02:26, GitBox wrote:
>> alpire opened a new pull request #176: CoyoteAdapter: fix
>> out-of-bounds read in checkNormalize URL:
>> https://github.com/apache/tomcat/pull/176
>> 
>> 
>> On malformed requests, checkNormalize would throw an
>> ArrayIndexOutOfBoundsException leading to a 500 response. This
>> change fixes checkNormalize to return false instead of throwing
>> exception on those inputs, and adds a few tests to check the new
>> functionality.
> 
> That there are URI sequences that can trigger this is certainly 
> something that we need to fix.
> 
> On a slightly different topic, this got me thinking.
> 
> checkNormalize() is a test that runs on every request. It is
> essentially there to ensure that whatever character set has been
> specified for the Connector is "safe". In this case "safe" means
> when the bytes are converted to characters we don't get any
> unexpected "." or "/" characters in the result that make the
> character version of the URI non-normalized.
> 
> Rather than run this test on every request, we could: -
> pre-calculate the character sets we consider to be safe - throw an
> exception if the user attempts to configure the Connector to use
> one
> 
> I think we could safely exclude any character set where any of the 
> following were not true:
> 
> - 0x00 <-> '\u0000' - 0x2e <-> '.' - 0x2f <-> '/' - 0x5c <->
'\'
> 
> We could create a unit test that maintains/checks the list of
> "safe" character set canonical names. After creating the character
> set in Connector.setURIEncoding(), if the canonical name of the
> resulting character set is not in the safe list, throw an
> exception.
> 
> Then remove checkNormalize() and replace with a comment that
> explains why the conversion is known to be safe.
> 
> There are several loops through the URI in checkNormalize(). I
> think - I'd need to test to confirm - that removing them would
> provide a measurable performance benefit.

I'm a little worried about edge cases that might cause a misread of a
multibyte character into several single-byte characters that aren't
supposed to be treated as such. Remember that an attacker is not
obligated to follow the rules of sane e.g. UTF-8 or SHIFT-JS or
whatever. They can claim one encoding and then send complete garbage
down the line. We need to remain safe in all cases.

So I think checking the string AFTER decoding is the only truly safe
way to operate.

Note that I can't think of a specific way to break this off the top of
my head.

What kind of performance benefit are you looking at, here? Certainly,
every bit helps, but is the wire-to-servlet pipeline really that long
at this point?

- -chris
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl0aIY8ACgkQHPApP6U8
pFhzrBAAwpIKVsiUIJiEQIEMsioeBSyShut/jncTjYNe1+JpdDWySDgQtuv6rzJm
ImMsi7/Iutm+OTvPyX/0ErkueYTwsdQTMS3dAGnyC+naViLQ+/C5XsOLIisHJwUy
suDJsVl5V0DbslXauhNiy+tsMzA8ipx87OneYO7gBMjcurLtMjvPiCnMvRS9eFah
C5bU6Emu9NrEC40MI7Jw+Il/loUIDlLbCDIqDjGxMb/tA8N0wXmIoIIbC1iAXPpy
fmSN4MyEUd0Xi4m/sv7kl71hmI09ivItHUtMdW89Tp8kvIEWUAnu3mDoLZGmw3+X
YZIxhW+95g4HydOh7ODuJa7Bqg+csa0ZpQl5u4jnnwGZrLWLGZ/w+tu5P83qHBp6
Yy1hdPVrhfAgLMV1Rmzj+5Heo0LGe2PbKvgpg4DMAFSed3SOnnW/w3NNPTjFhEBD
jiQi1TSXTZZQ6VRS/KWain4uQAWD2ouPipZl7XA8Uz+g0pEhh642s7LNUpbclH5y
JZ02GRL5GFAcb9ZTn0RlCQgDz/xK9Lm5eVbHmWkdtHw9CUgqWF9nD6ZhWXEEI39Z
vVQZboAZMlkijv3AJupjE6Q8PfPnXWaFdbtDqWMJQ+/c12xzPSGu+QcpgCYxyIfE
6kSig2FGR98lKXCE5bMMq6XuM1RNcHMyKUnOZ8NOxAhLfBbZcEw=
=zgdi
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message