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.

Sign up for GitHub

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

Jump to bottom

Crash at v8::Object::GetRealNamedPropertyAttributes #34606

Closed
po6ix opened this issue Aug 3, 2020 · 1 comment
Closed

Crash at v8::Object::GetRealNamedPropertyAttributes #34606

po6ix opened this issue Aug 3, 2020 · 1 comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. confirmed-bug Issues with confirmed bugs. v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem.

Comments

@po6ix
Copy link

po6ix commented Aug 3, 2020

  • Version:
    v14.6.0

  • Platform:
    ubuntu 19.04

What steps will reproduce the bug?

const vm = require('vm');

var handler = {
    getOwnPropertyDescriptor: () => {
        return {};
    }
};

const source = `p=6`;

var proxy = new Proxy({}, handler);
const ctx = vm.createContext(proxy);

script = new vm.Script(source);
script.runInContext(ctx);

How often does it reproduce? Is there a required condition?

nothing

What is the expected behavior?

no error

What do you see instead?

crash

image
attachment core.zip
@po6ix po6ix changed the title SigABRT in v8::Object::GetRealNamedPropertyAttributes Crash in v8::Object::GetRealNamedPropertyAttributes Aug 3, 2020
@po6ix po6ix changed the title Crash in v8::Object::GetRealNamedPropertyAttributes Crash at v8::Object::GetRealNamedPropertyAttributes Aug 3, 2020
@addaleax addaleax added c++ Issues and PRs that require attention from people who are familiar with C++. confirmed-bug Issues with confirmed bugs. vm Issues and PRs related to the vm subsystem. v8 engine Issues and PRs related to the V8 dependency. labels Aug 3, 2020
@addaleax
Copy link
Member

addaleax commented Aug 3, 2020

Opened https://chromium-review.googlesource.com/c/v8/v8/+/2335057 to address this in V8

pull bot pushed a commit to p-g-krish/v8 that referenced this issue Aug 6, 2020
@addaleax
[api] Fix empty Maybe crash in GetRealNamedPropertyAttributes
e06ace6 
`Object::GetRealNamedPropertyAttributes()` can crash if an empty
`Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because
it was not checking for that. Fix that.

Refs: nodejs/node#34606
Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69258}
addaleax added a commit to addaleax/node that referenced this issue Aug 7, 2020
@addaleax
deps: V8: cherry-pick e06ace6b5cdb
2b0a451 
Original commit message:

    [api] Fix empty Maybe crash in GetRealNamedPropertyAttributes

    `Object::GetRealNamedPropertyAttributes()` can crash if an empty
    `Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because
    it was not checking for that. Fix that.

    Refs: nodejs#34606
    Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69258}

Refs: v8/v8@e06ace6
addaleax added a commit to addaleax/node that referenced this issue Aug 7, 2020
@addaleax
test: add vm crash regression test
bd0ccf2 
Fixes: nodejs#34606
@addaleax addaleax mentioned this issue Aug 7, 2020
Closed
3 tasks
addaleax added a commit that referenced this issue Aug 10, 2020
@addaleax
test: add vm crash regression test
64acae2 
Refs: #34606
PR-URL: #34673
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Aug 17, 2020
@addaleax @MylesBorins
deps: V8: cherry-pick e06ace6b5cdb
5b2a1f1 
Original commit message:

    [api] Fix empty Maybe crash in GetRealNamedPropertyAttributes

    `Object::GetRealNamedPropertyAttributes()` can crash if an empty
    `Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because
    it was not checking for that. Fix that.

    Refs: #34606
    Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69258}

Refs: v8/v8@e06ace6

PR-URL: #34673
Fixes: #34606
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Aug 17, 2020
@addaleax @MylesBorins
test: add vm crash regression test
e4d3dbd 
Refs: #34606
PR-URL: #34673
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Aug 20, 2020
@addaleax @BethGriggs
deps: V8: cherry-pick e06ace6b5cdb
9527a2a 
Original commit message:

    [api] Fix empty Maybe crash in GetRealNamedPropertyAttributes

    `Object::GetRealNamedPropertyAttributes()` can crash if an empty
    `Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because
    it was not checking for that. Fix that.

    Refs: #34606
    Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69258}

Refs: v8/v8@e06ace6

PR-URL: #34673
Fixes: #34606
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Aug 20, 2020
@addaleax @BethGriggs
test: add vm crash regression test
27c0653 
Refs: #34606
PR-URL: #34673
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this issue Sep 7, 2020
@addaleax @targos
deps: V8: cherry-pick e06ace6b5cdb
148a8fb 
Original commit message:

    [api] Fix empty Maybe crash in GetRealNamedPropertyAttributes

    `Object::GetRealNamedPropertyAttributes()` can crash if an empty
    `Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because
    it was not checking for that. Fix that.

    Refs: nodejs#34606
    Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69258}

Refs: v8/v8@e06ace6

PR-URL: nodejs#34673
Fixes: nodejs#34606
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit that referenced this issue Sep 27, 2020
@addaleax
deps: V8: cherry-pick e06ace6b5cdb
5a99045 
Original commit message:

    [api] Fix empty Maybe crash in GetRealNamedPropertyAttributes

    `Object::GetRealNamedPropertyAttributes()` can crash if an empty
    `Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because
    it was not checking for that. Fix that.

    Refs: #34606
    Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69258}

Refs: v8/v8@e06ace6

PR-URL: #34673
Fixes: #34606
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit that referenced this issue Sep 27, 2020
@addaleax
test: add vm crash regression test
20ab2c7 
Refs: #34606
PR-URL: #34673
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit that referenced this issue Sep 28, 2020
@addaleax
deps: V8: cherry-pick e06ace6b5cdb
4ff6c77 
Original commit message:

    [api] Fix empty Maybe crash in GetRealNamedPropertyAttributes

    `Object::GetRealNamedPropertyAttributes()` can crash if an empty
    `Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because
    it was not checking for that. Fix that.

    Refs: #34606
    Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69258}

Refs: v8/v8@e06ace6

PR-URL: #34673
Fixes: #34606
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit that referenced this issue Sep 28, 2020
@addaleax
test: add vm crash regression test
3598056 
Refs: #34606
PR-URL: #34673
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/abi-stable-v8 that referenced this issue Apr 17, 2021
@addaleax @targos
[api] Fix empty Maybe crash in GetRealNamedPropertyAttributes
5ddd611 
`Object::GetRealNamedPropertyAttributes()` can crash if an empty
`Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because
it was not checking for that. Fix that.

Refs: nodejs/node#34606
Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69258}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. confirmed-bug Issues with confirmed bugs. v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

deps: V8: cherry-pick e06ace6b5cdb addaleax/node
2 participants
@addaleax @po6ix