ср, 15 мар. 2023 г. в 13:15, Konstantin Kolinko <knst.koli...@gmail.com>: > > ср, 15 мар. 2023 г. в 12:07, Mark Thomas <ma...@apache.org>: > > > > On 14/03/2023 21:13, Christopher Schultz wrote: > > > Mark, > > > > > > On 3/14/23 13:57, Mark Thomas wrote: > > >> On 09/03/2023 14:23, Christopher Schultz wrote: > > >>> Mark, > > >>> > > >>> On 3/9/23 05:56, Mark Thomas wrote: > > >>>> Hi all, > > >>>> > > >>>> In the context of CVE-2023-24998 (performance issues for large > > >>>> numbers of uploaded parts), I have been wondering about reducing the > > >>>> default value for maxParameterCount. > > >>>> > > >>>> The current default for maxParameterCount is 10,000. It was set > > >>>> based on it being low enough to mitigate CVE-2012-0022 (hash > > >>>> collisions in parameter names triggering performance issues) while > > >>>> being so high it was considered extremely unlikely to impact any web > > >>>> application. > > >>> > > >>> Also relevant: maxPostSize and maxHttpRequestHeaderSize which help to > > >>> limit the total size of a request, regardless of the number of > > >>> parameters. > > >> > > >> I don't think we can lower those any further by default. If anything, > > >> the trend is towards making them larger. > > >> > > >>>> The current default is sufficiently low to mitigate CVE-2023-24998. > > >>>> > > >>>> There isn't any reason I am aware of that means we need to reduce > > >>>> the default for maxParameterCount. My thinking is more along the > > >>>> lines that when we last thought about this default in 2012, it was > > >>>> considered from the perspective of "How high can we set this and > > >>>> still be sure applications aren't exposed to CVE-2012-0022 or > > >>>> something like it?". If we consider it from the perspective of "How > > >>>> low can we make this without breaking many / most / (nearly) all > > >>>> applications?" I think we'll choose a much lower number. > > >>> > > >>> +1 > > >>> > > >>>> Another benefit of a lower number is to harden Tomcat in advance > > >>>> against future vulnerabilities like CVE-2023-24998. > > >>>> > > >>>> I was wondering about a new default of 1000 or maybe even 500. > > >>>> > > >>>> This would certainly be for 11.0.x. I think it should be back-ported > > >>>> but maybe in stages (5000, 3000, 2000, 1000) and/or delayed so it is > > >>>> reduced in 10.1.x for a few releases before we reduce it in 9.0.x > > >>>> and the a few more releases before we reduce it in 8.5.x. > > >>>> > > >>>> Thoughts? > > >>> > > >>> +1 for 1000. 500 seems insane to me but I'm sure there is some > > >>> application out there which uses 1000 parameters instead of JSON, > > >>> etc. for some reason. > > >> > > >> I've reduced the default to 1,000 for 11.0.x. > > >> > > >> Thoughts on if/how to back-port this to 10.1.x and friends? > > >> > > >> Straight to 1000 for all older versions? > > >> Straight to 1000 for 10.1.x then wait a few releases for each further > > >> backport? > > >> Or more cautious and backport a gradual reduction? > > > > > > I would go for a 1000 limit for all currently-supported versions. It's > > > *very* easy to raise the limit if it interferes with a specific > > > application's functions. > > > > > > I *would* add an entry in the "notable changes" for each release e.g. > > > https://tomcat.apache.org/migration-10.1.html#Tomcat_10.1.x_noteable_changes > > > > Makes sense. > > > > I'll do that. > > -1 unless the behaviour of "silently dropping extra parameters" is > changed as well. > > Silent loss of data is not what I want to see in production. > > Documentation [1] says "Request parameters beyond this limit will be ignored." > [1] https://tomcat.apache.org/tomcat-8.5-doc/config/http.html > > More details shortly.
First, some notes regarding the source code. (1) The limit is enforced in org.apache.tomcat.util.http.Parameters#addParameter(). It throws an IllegalStateException which is either caught and retrown later, or caught and swallowed. (2) I see the following 4 code paths and cases: a) Request.getParameter() was called, and request was of type "application/x-www-form-urlencoded". - Code path: org.apache.tomcat.util.http.Parameters#addParameter() is called from org.apache.tomcat.util.http.Parameters#processParameters(...) In Parameters#processParameters(...) the IllegalStateException and IOException are caught, logged with UserDataHelper and swallowed. So the error is logged. b) Request.getPart(), and request was "application/x-www-form-urlencoded". JavaDoc for HttpServletRequest.#getPart() says that this is not allowed, and the expected result is a ServletException. c) Request.getPart(), and request was "multipart/form-data". - Code path: org.apache.tomcat.util.http.Parameters#addParameter() is called from org.apache.catalina.connector.Request.parseParts(...) is called from org.apache.catalina.connector.Request.getPart(...) Request.parseParts(...) catches the ISE, stores it. Request.getPart(...) rethrows the caught ISE. The API of Request.getPart(...) allows rethrowing the errors. So the error is visible to the caller. d) Request.getParameter() was called, and request was "multipart/form-data". - Code path: org.apache.tomcat.util.http.Parameters#addParameter() is called from org.apache.catalina.connector.Request.parseParts(...) is called from org.apache.catalina.connector.Request.getParameter(...) Request.parseParts(...) catches the ISE, stores it. So the error is not visible, and is not logged. (3) Another situation that might occur is decoding error, or generally an IOException. It is generally handled in the same way. Proposals shortly. Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
