Skip to content

Conversation

@thymikee
Copy link
Member

@thymikee thymikee commented Nov 12, 2025

Summary

Continuation of the fix that landed in 1508990, that prevents RCE using a spoofed URL with | character, such as: https://evil.com?|calc.exe.

cc @633kh4ck @mbaraniak-exodus

@mbaraniak-exodus
Copy link

@thymikee,
The fix seems reasonable, unless you switch in the future to a new version of open, which uses PS underneath. Then you will need escape also $ (, etc.
Be aware of (non-default) delayed expansion, which will make such syntax possible !VAR!

}

// Reconstruct URL with proper encoding to prevent command injection
// The URL constructor doesn't automatically encode special characters like | in query strings,
Copy link

@633kh4ck 633kh4ck Nov 12, 2025

Choose a reason for hiding this comment

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

To be specific, it encodes special characters, but only sets of them in each URL part 1. For example, | is encoded in userinfo:

new URL('https://user|:[email protected]')`
// https://user%7C:[email protected]/

Current implementation double-encodes several characters for that reason; for example, whitespaces:

const parsedUrl = new URL('https://example.com/?#some hash')
// https://example.com/?#some%20hash
const sanitizedUrl = new URL(parsedUrl.origin);
// ...
console.log(sanitizedUrl.href)
// https://example.com/#some%2520hash

A simpler approach could be:

const sanitizedUrl = encodeURI(url);

Footnotes

  1. https://url.spec.whatwg.org/#percent-encoded-bytes

jest.restoreAllMocks();
});

it('should sanitize URL with pipe character to prevent RCE', async () => {

Choose a reason for hiding this comment

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

@633kh4ck
Copy link

This is likely still fragile, but better than it was.

On a side note, this can (still) be exploited to exfiltrate some environment variables; possibilities are more limited, though. For example, https://example.com/?a=%¾TA% is encoded to https://example.com/?a=%25%C2%BETA%25 (note %BETA%).

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants