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

TypeError: decodeComponents(...).join is not a function #345

Closed
KatsuragiCSL opened this issue Aug 24, 2022 · 12 comments
Closed

TypeError: decodeComponents(...).join is not a function #345

KatsuragiCSL opened this issue Aug 24, 2022 · 12 comments

Comments

@KatsuragiCSL
Copy link

Version: v7.1.1

Crash occured when # kun%ea%ba%5a%ba is parsed by queryString.parse
This hash is valid and should be parsed correctly as { ' kun%ea%ba%5a%ba': null }
For example in chrome's development tools' console the url https://google.com# kun%ea%ba%5a%ba can be parsed without problems:

const url = new URL("https://google.com#  kun%ea%ba%5a%ba");
url

image

Code to reproduce

const queryString = require('query-string');

const parsed = queryString.parse("#  kun%ea%ba%5a%ba");
console.log(parsed);

Results

TypeError: decodeComponents(...).join is not a function

@sindresorhus
Copy link
Owner

Must be fixed in SamVerschueren/decode-uri-component#5

@wzijden
Copy link

wzijden commented Nov 29, 2022

Is it an option to simply get rid of this dependency and use the built in decodeURIComponent instead?

@Prophet32j
Copy link

my advice is to ditch the vulnerable library. The issue was identified on the project in August and hasn't been fixed yet. The project hasn't had any meaningful activity on it in 3 years. Either fork and fix and use the forked repo or migrate away from it. We are seeing this as well, too.

@eddilou
Copy link

eddilou commented Nov 30, 2022

Is it an option to simply get rid of this dependency and use the built in decodeURIComponent instead?
I will upset you, but even in the decodeURIComponent it does not work correctly

@wzijden
Copy link

wzijden commented Nov 30, 2022

I looked at this library and all it does is replacing + with a space, and only if the native decodeURIComponent throws an error, it attempts to parse the string itself. So anything it crashes on is something that decodeURIComponent also crashes on.

I guess I anyway don't understand why this is considered a vulnerability.

@lexek
Copy link

lexek commented Nov 30, 2022

@wzijden apparentely, someone decided it would be brilliant idea to start filing any unexpected exceptions caused by input (which even in very far fetched cases might be inputted by remote user) in libraries as DoS vulnerabilities. Not the first I see this kind of reasoning

@SamVerschueren
Copy link
Contributor

I'll try to get a release out today.

People see to be very friendly nowadays. Piling up my Twitter DMs because Snyk seems to mark this as high severity...

@eddilou
Copy link

eddilou commented Dec 1, 2022

I'll try to get a release out today.

People see to be very friendly nowadays. Piling up my Twitter DMs because Snyk seems to mark this as high severity...

The problem is that this is not your vulnerability, but the vulnerability of a native function, which for some reason was shifted to you

@eddilou
Copy link

eddilou commented Dec 1, 2022

I'll try to get a release out today.

People see to be very friendly nowadays. Piling up my Twitter DMs because Snyk seems to mark this as high severity...

I understand, there you need another try-catch block

@mahesh-kavya
Copy link

I see that decode-uri-component is upgraded to 0.2.2, but it also has the same issue

@SamVerschueren
Copy link
Contributor

Have you actually tried it?

https://stackblitz.com/edit/node-fhizma?file=index.js

@sindresorhus I think you can close this issue now.

@SamVerschueren
Copy link
Contributor

Regarding the Snyk "vulnerability". I'm in contact with them to see what needs to happen but that is unrelated to this issue.

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

No branches or pull requests

8 participants