You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@struts.apache.org by "henrikplate (via GitHub)" <gi...@apache.org> on 2025/01/07 09:25:24 UTC
[PR] Fix CVE-2024-53677 in 2-3-x [struts]
henrikplate opened a new pull request, #1180:
URL: https://github.com/apache/struts/pull/1180
The previous fix for CVE-2023-50164 (see #819) removed existing parameters if their names were equal (case-insensitive) to parameters related to file uploads, e.g. `uploadFileName` vs. `UploadFileName`.
However, the previous fix was incomplete, because it does not catch other OGNL expressions that could also be used by attackers to override the file name of upload actions, e.g. `top.uploadFileName`.
The fix proposed in this PR overcomes this problem by checking for containment rather than equality, i.e. any existing parameter that contains e.g. `uploadFileName` (case-insensitive) will be removed before adding a legitimate `UploadFileName`.
This check is only enabled for the two parameters related to file name and content-type. It is not enabled for the file itself, e.g. parameter `Upload`, since this could potentially result in the removal of other legitimate parameters, e.g. `UploadDescription`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix CVE-2024-53677 in 2-3-x [struts]
Posted by "henrikplate (via GitHub)" <gi...@apache.org>.
henrikplate commented on PR #1180:
URL: https://github.com/apache/struts/pull/1180#issuecomment-2574968279
@lukaszlenart Thank you for the quick feedback, much appreciated. I knew that there will be no new release, and that users are advised to migrate to 6.4.0 or later. The main idea of this PR is to buy people struggling with the migration some time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix CVE-2024-53677 in 2-3-x [struts]
Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on PR #1180:
URL: https://github.com/apache/struts/pull/1180#issuecomment-2574915976
@henrikplate I can merge the PR but 2.3.x is EOL and there no new release of it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix CVE-2024-53677 in 2-3-x [struts]
Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on PR #1180:
URL: https://github.com/apache/struts/pull/1180#issuecomment-2575787494
Ah, right! Coping the rest should be a problem but you are right it can be too much for most of the users. Hope you can migrate to Struts 6.x soon 🙏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix CVE-2024-53677 in 2-3-x [struts]
Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart merged PR #1180:
URL: https://github.com/apache/struts/pull/1180
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix CVE-2024-53677 in 2-3-x [struts]
Posted by "henrikplate (via GitHub)" <gi...@apache.org>.
henrikplate commented on PR #1180:
URL: https://github.com/apache/struts/pull/1180#issuecomment-2602408850
fyi @lukaszlenart — I thought it is worthwhile to add that this fix is part of [Endor Labs](https://www.endorlabs.com/)' efforts to create minimal invasive, yet viable security fixes for older OSS versions (similar to the well-known and established approach followed by various Linux distributions).
I haven’t come up with a standard disclaimer/background section when I created this PR, but I am adding this info (and a bit more) on other PRs going forward. Here’s an example from the GitHub repo [haraldk/TwelveMonkeys/pull/1080](https://github.com/haraldk/TwelveMonkeys/pull/1080).
Thank you again, Henrik
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix CVE-2024-53677 in 2-3-x [struts]
Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on PR #1180:
URL: https://github.com/apache/struts/pull/1180#issuecomment-2575098974
@henrikplate other easier approach is to implement exactly the same `ActionFileUploadInterceptor` in your app. I assume you can copy paste existing code from 6.4.0 code base.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix CVE-2024-53677 in 2-3-x [struts]
Posted by "henrikplate (via GitHub)" <gi...@apache.org>.
henrikplate commented on PR #1180:
URL: https://github.com/apache/struts/pull/1180#issuecomment-2575775526
@lukaszlenart I copied the latest `ActionFileUploadInterceptor` from the 6-7-x branch (as well as the version from tag STRUTS_6_4_0) into the 2-3-x branch, but compilation fails due to missing symbols like `UploadedFile`, `UploadedFilesAware` or `AbstractFileUploadInterceptor` as well as differences in regard to logging.
This makes me think that to much refactoring happened between those releases to make down-port of `ActionFileUploadInterceptor` feasible. In other words, the local fix proposed in this PR still seems an easier option to me.
Ideally, users should be able to use this as a drop-in replacement for the vulnerable version 2.3.37, without any need for changing their application or updating other dependencies. Again, I know that updating to a supported version of Struts is the recommended long-term solution, but this local fix could be a short-term remedy.
Of course, this only works if the proposed fix both
- eliminates the possibility for attackers to override the file name by fiddling with the parameters’ sort order, e.g. using lower case or OGNL expressions (there may be other ways than the `top.` prefix and index access via `[0]` mentioned above and in blog posts), and
- it is not too restrictive to break any existing, legitimate scenarios (which is why I limited the check to the `<name>ContentType` and `<name>FileName` parameters).
It would love to hear your assessment of those two aspects, thank you in advance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org