Skip to content
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

Encode the path in uri #23907

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adkharat
Copy link
Contributor

@adkharat adkharat commented Oct 29, 2024

Description

CWE-235

Fixed Http Parameter Pollution Rule at :

(1) Encoded URI path parameter /v1/taskInfo/{taskId} with UTF_8 encoding : presto-main/src/main/java/com/facebook/presto/resourcemanager/DistributedTaskInfoResource.java:72
(2) presto-main/src/main/java/com/facebook/presto/server/protocol/QueryResourceUtil.java

Motivation and Context

Concatenating untrusted input into path, query string, or parameters for HTTP message (in the URL or in the request body) may allow an attacker to override/add unexpected request parameters. Attacker may change the intended HTTP request semantics, and may give him/her access to unauthorized data or bypass web application firewall validations.

Impact

No Impact

Test Plan

Verified existing Unit test case for above fix :
(1) presto-tests/src/test/java/com/facebook/presto/resourcemanager/TestDistributedTaskInfoResource.java
(2) presto-tests/src/test/java/com/facebook/presto/server/TestQueryResource.java

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

* Security Changes
 Fixes URI path parameter encoding to handle untrusted input : pr:`23907`

@adkharat adkharat requested a review from a team as a code owner October 29, 2024 12:35
@adkharat adkharat marked this pull request as draft October 29, 2024 15:56
@tdcmeehan tdcmeehan self-assigned this Oct 29, 2024
@adkharat adkharat force-pushed the static_high_http_pollution branch 2 times, most recently from 1bc6b9d to 84fedb7 Compare October 30, 2024 11:46
@adkharat adkharat marked this pull request as ready for review October 30, 2024 14:26
@adkharat adkharat changed the title Encode the path from uriInfo Encode the path in uri Oct 30, 2024
@tdcmeehan
Copy link
Contributor

Instead of UTF-8 encoding the task and other elements on the path, can't we simply disallow any additional query parameters? The task ID is validated, can't we simply disallow any other parameters.

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.

2 participants