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

Null Pointer Dereference in build_for_in_iterator #192

Closed
anbu1024 opened this issue Sep 1, 2023 · 1 comment
Closed

Null Pointer Dereference in build_for_in_iterator #192

anbu1024 opened this issue Sep 1, 2023 · 1 comment

Comments

@anbu1024
Copy link

anbu1024 commented Sep 1, 2023

QuickJS version:
2788d71

Test case:

function foo() {
    
    function Bar() {
    }
    
    class Apple extends Bar {
        constructor(a) {
            (() => {
            	for (const i in this) {}
                eval(a);
                return 0;
            })();
        }
    }
    const y = new Apple();
    return y;
}

let x = new Promise(foo);

Error:
crashed due to null pointer deference,

In function JSValue build_for_in_iterator(JSContext *ctx, JSValue obj), line 15129, the

p = JS_VALUE_GET_OBJ(obj);

returns 0.

@dchest
Copy link

dchest commented Sep 1, 2023

I believe this is caused by eval appearing in the function causing the function to close over this, but not keeping the is_lexical status here :

https://github.com/bellard/quickjs/blob/2788d71e823b522b178db3b3660ce93689534e6d/quickjs.c#L30312-L30320

                vd = &fd->vars[i];
                /* do not close top level last result */
                if (vd->scope_level == 0 &&
                    vd->var_name != JS_ATOM__ret_ &&
                    vd->var_name != JS_ATOM_NULL) {
                    get_closure_var(ctx, s, fd,
                                    FALSE, i, vd->var_name, FALSE, FALSE,
                                    JS_VAR_NORMAL);
                }

is_lexical is added in order to emit opcode that checks that variable is initialized:

https://github.com/bellard/quickjs/blob/master/quickjs.c#L29514-L29515

without it, accessing an uninitialized variable is done without checking, so it crashes.

Possible fix: add vd->var_name != JS_ATOM_this to the above construction inside add_eval_variables to skip adding this (since it's already added earlier), but I'm not sure if it's correct.

@bellard bellard closed this as completed Dec 9, 2023
TooTallNate pushed a commit to TooTallNate/quickjs that referenced this issue Dec 18, 2023
Commit f404980 ("Add fused get_loc0_loc1 opcode") introduced an
off-by-one (sometimes negative) array index bug because OP_get_loc1_loc1
replaced OP_get_loc0 as the first OP_FMT_none_loc opcode.
TooTallNate pushed a commit to TooTallNate/quickjs that referenced this issue Dec 18, 2023
GerHobbelt pushed a commit to GerHobbelt/quickjs that referenced this issue Jan 26, 2024
daa35bc new release
a057008 added Array.prototype.findLast{Index} and TypeArray.prototype.findLast{index} (initial patch by bnoordhuis)
177af41 fixed duplicate static private setter/getter test
b180cd2 Symbol.species is no longer used in TypedArray constructor from a TypedArray
e182050 fixed delete super.x error
58f374e reworked set property and fixed corner cases of typed array set property
20a57f9 Implement extended named capture group identifiers (bnoordhuis)
4949d75 Retrieve RegExp 'g' flag in spec conformant way (original patch by bnoordhuis)
c4cdd61 fixed lexical scope of 'this' with eval (github issue bellard#192)
26fdf65 Make Date methods argument coercion spec compliant (bnoordhuis)
b14d77b fixed negative zero date
55a4878 fixed private field setters (github issue bellard#194)
321dbfa added missing bignum error tests (github issue bellard#159)
f87cab0 added String.prototype.at, Array.prototype.at and TypedArray.prototype.at
3106401 keep LTO
cdeca4d updated to unicode 15.0.0
94010ed the BigInt support is now always included
03cc5ec fixed js_proxy_isArray stack overflow (github issue bellard#178)
6de52d8 bf_set_ui() fix (github issue bellard#133)
2788d71 updated to Unicode 14.0.0
8516959 updated test262.conf
446099a added Object.hasOwn()
b9f5880 fixed invalid Array.prototype.push/unshift optimization

git-subtree-dir: quickjs
git-subtree-split: daa35bc
xplshn pushed a commit to xplshn/Mirror-of-the-Chawan-web-browser that referenced this issue Apr 9, 2024
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

3 participants