-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ssrf fix #3444
Ssrf fix #3444
Conversation
Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Co-authored-by: Kevin van Rijn <6368561+kevinvanrijn@users.noreply.github.com>
* security: patch request for CVE-2023-28155 GHSA-p8p7-x288-28g6 Ported from request/request#3444 * add iyarc exclusion
* security: patch request for CVE-2023-28155 GHSA-p8p7-x288-28g6 Ported from request/request#3444 * add iyarc exclusion
* security: patch request for CVE-2023-28155 GHSA-p8p7-x288-28g6 Ported from request/request#3444 * add iyarc exclusion
Can we expect an updated request release with this fix soon? |
There were no non-doc/tools changes in the repo since 2018 and no commits at all for the last 3 years. Our own dependency on Update: in case somebody missed that, |
Yeah... It seems like it's not being maintained/updated anymore a long time. |
Thanks. |
@khitrenovich CVE-2023-28155 was reported on Mar 16, 2023 and the last commit in fork from Cypress is from Jan 11, 2023. It doesn't look like this has been fixed in their fork 🤨 |
@suside You are right... Just opened an issue there, let's see how they respond. |
@SzymonDrosdzol it might be worth you opening the same CVE against the cypress fork too |
- Addresses CVE-2023-28155 - Existing behavior allows malicios redirects between protocols - Set default behavior to disable this vector (breaking) - Add new option `allowInsecureRedirect` where `true` reverts to old behavior - Ported from request#3444
I have made two PRs on cypress:
It seems they have an extended test suite handling more complex redirects so if anyone feels up for extending my PRs addressing those cases to get them up to standard, feel free. |
is there any alternate library of sforcejs where we do not have dependency on request library ? |
It would be great if a new version could be released including the fix. I see there was a new version a couple of days ago with the new version of xml2js so I have hope. |
@jeremieSTC A couple of years, you mean? See above for alternatives - request itself is unmaintained and I wouldn't be hoping to see further updates. |
Per the readme and npm deprecation warning.
Please use alternative libraries. |
- Addresses CVE-2023-28155 - Existing behavior allows malicios redirects between protocols - Set default behavior to disable this vector (breaking) - Add new option `allowInsecureRedirect` where `true` reverts to old behavior - Ported from request#3444
- Addresses CVE-2023-28155 - Existing behavior allows malicios redirects between protocols - Set default behavior to disable this vector (breaking) - Add new option `allowInsecureRedirect` where `true` reverts to old behavior - Ported from request#3444
PR Checklist:
npm test
locally and all tests are passing.PR Description
This pull request is a fix to CVE-2023-28155.
It introduces a new configuration option
allowInsecureRedirects
, turned off by default. The default configuration leaves library users protected from exploiting CVE-2023-28155. When the option gets turned on, the cross-protocol redirects will be allowed if library user decides it's safe and required in their case.