ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.73k stars 518 forks source link

upgrade of Apache Commons FileUpload to 1.5 #476

Closed jefimm closed 1 year ago

jefimm commented 1 year ago

https://nvd.nist.gov/vuln/detail/CVE-2023-24998

jefimm commented 1 year ago

https://lists.apache.org/thread/4xl4l09mhwg4vgsk7dxqogcjrobrrdoy

svdm commented 1 year ago

Besides bumping commons-fileupload to 1.5 this needs a code change to use the new FileUpload.setFileCountMax API to actually configure a reasonable limit, because there is no default. What's reasonable is going to differ, so this in turn may need a config option on the multipart-params middleware.

weavejester commented 1 year ago

Thanks for the report. I'll upgrade commons-fileupload to 1.5, and add :max-file-count, :max-file-size and :max-body-size options to the multipart middleware.

If any of the limits are hit, the middleware will return a customizable 413 Content Too Large response.

weavejester commented 1 year ago

It looks like using the setFileCountMax method isn't possible as it requires a RequestContext, so I'll need to put together some custom checks.

svdm commented 1 year ago

Oh, you're right, setFileCountMax is only used in parseRequest. To do the checks in the middleware will require tracking some state while building a seq from the FileItemIterator (probably simplest to just reduce over it, the current laziness is consumed eagerly in parse-multipart-params AFAICT).

weavejester commented 1 year ago

The laziness in the seq is important, because it's wrapping an iterator of an input stream. We don't know how many files there are until the body input stream has been consumed.

eudoroolivares2016 commented 1 year ago

This is an issue on previous versions prior to 1.9.6 as well as 1.9.6 correct?

weavejester commented 1 year ago

Yes. It will be "fixed" in 1.10.0, insofar that an option will be allowed that limits the maximum number of files allowed in a request.

TahmurasAbdurashidovTR commented 1 year ago

Is there any release date for 1.10.0?

weavejester commented 1 year ago

Is there any release date for 1.10.0?

It should be within a few days. I've updated the multipart middleware and it passes all the tests. I decided not to include a :max-body-size option as that could be a separate middleware.

eudoroolivares2016 commented 1 year ago

Is there a link I haven't seen to version 1.10.0, perhaps I'm looking in the wrong place: https://mvnrepository.com/artifact/ring/ring-core?

weavejester commented 1 year ago

Released 1.10.0. Took a little longer to find time than I expected due to unforeseen circumstances.

weavejester commented 1 year ago

Is there a link I haven't seen to version 1.10.0, perhaps I'm looking in the wrong place: https://mvnrepository.com/artifact/ring/ring-core?

https://clojars.org/ring/ring-core