Parsing uploaded file parameter name from Content-Disposition header fails 2.2.6.3 update #2076
Assignees
Comments
jeremyevans
added a commit
to jeremyevans/rack
that referenced
this issue
Apr 28, 2023
The ReDoS fix in ee25ab9 breaks valid requests, because colons are valid inside parameter values. You cannot use a regexp scan and ensure correct behavior, since values inside parameters can be escaped. Issues like this are the reason for the famous "now they have two problems" quote regarding regexps. Add a basic parser for parameters in Content-Disposition. This parser is based purely on String#{index,slice!,[],==}, usually with string arguments for #index (though one case uses a simple regexp). There are two loops (one nested in the other), but the use of slice! ensures that forward progress is always made on each loop iteration. In addition to fixing the bug introduced by the security fix, this removes multiple separate passes over the mime head, one pass to get the parameter name for Content-Disposition, and a separate pass to get the filename. It removes the get_filename method, though some of the code is kept in a smaller normalize_filename method. This removes 18 separate regexp contents that were previously used just for the separate parse to find the filename for the content disposition. Fixes rack#2076
jeremyevans
added a commit
that referenced
this issue
Apr 28, 2023
The ReDoS fix in ee25ab9 breaks valid requests, because colons are valid inside parameter values. You cannot use a regexp scan and ensure correct behavior, since values inside parameters can be escaped. Issues like this are the reason for the famous "now they have two problems" quote regarding regexps. Add a basic parser for parameters in Content-Disposition. This parser is based purely on String#{index,slice!,[],==}, usually with string arguments for #index (though one case uses a simple regexp). There are two loops (one nested in the other), but the use of slice! ensures that forward progress is always made on each loop iteration. In addition to fixing the bug introduced by the security fix, this removes multiple separate passes over the mime head, one pass to get the parameter name for Content-Disposition, and a separate pass to get the filename. It removes the get_filename method, though some of the code is kept in a smaller normalize_filename method. This removes 18 separate regexp contents that were previously used just for the separate parse to find the filename for the content disposition. Fixes #2076
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There's a fix for ReDos vulnerability in
ee25ab9
which breaks the Content-Disposition parameter name parsing in case header contains extra attributes like modification-date.
I.e. this works, parameter name is considered "file"
Content-Disposition: form-data; name="file"; filename="sample.sql"
However, in this case it fails to parse parameter name and file name itself is used as parameter name:
Content-Disposition: form-data; filename="sample.sql"; modification-date="Wed, 26 Apr 2023 11:01:34 GMT"; size=24; name="file"
The text was updated successfully, but these errors were encountered: