Skip to content

fix(node): enforce maxContentLength for data: URLs#7011

Merged
jasonsaayman merged 3 commits intoaxios:v1.xfrom
AmeerAssadi:advisory-fix-1
Sep 10, 2025
Merged

fix(node): enforce maxContentLength for data: URLs#7011
jasonsaayman merged 3 commits intoaxios:v1.xfrom
AmeerAssadi:advisory-fix-1

Conversation

@AmeerAssadi
Copy link
Contributor

Fix (Node): apply maxContentLength to data: URLs with pre-decode size check

  • Previously, data: payloads were fully decoded in memory and ignored maxContentLength > possible OOM/DoS.
  • Now, if maxContentLength is a finite non-negative value, Axios estimates the decoded size and rejects early when it exceeds the cap.
  • Default behavior unchanged (maxContentLength: -1 means unlimited).
  • Handles base64 (incl. padding/%3D) and non-base64 (UTF-8 byteLength upper bound).
  • No large allocations on the guarded path, error mirrors HTTP oversize handling.

Security: mitigates DoS.
See GHSA: GHSA-4hjh-wcwx-xvwjCVE-2025-58754.

@AmeerAssadi
Copy link
Contributor Author

@jasonsaayman plz approve!

@AmeerAssadi AmeerAssadi changed the title fix(node): enforce maxContentLength for data: URLs (CVE-2025-58754) fix(node): enforce maxContentLength for data: URLs Sep 9, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses a security vulnerability by enforcing maxContentLength limits on data: URLs to prevent denial-of-service attacks through out-of-memory conditions.

  • Adds pre-decode size estimation for data: URLs to check against maxContentLength before decoding
  • Implements efficient size calculation for both base64 and non-base64 encoded data without large memory allocations
  • Maintains backward compatibility by only enforcing limits when maxContentLength is explicitly set to a finite non-negative value

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Collaborator

@DigitalBrainJS DigitalBrainJS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution. 👍 Although the code looks like it works, it would be nice to add at least one unit test for the utility.

@AmeerAssadi
Copy link
Contributor Author

@DigitalBrainJS , thanks for the review, I have implemented the changes.
@jasonsaayman FYI

Copy link
Collaborator

@DigitalBrainJS DigitalBrainJS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@jasonsaayman jasonsaayman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🔥

@jasonsaayman jasonsaayman merged commit 945435f into axios:v1.x Sep 10, 2025
11 checks passed
@github-actions github-actions bot mentioned this pull request Sep 11, 2025
@github-actions
Copy link
Contributor

Hi, @AmeerAssadi! This PR has been published in v1.12.0 release. Thank you for your contribution ❤️!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants