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

Does BDSA-2023-1613 (CVE-2023-3420) impacts Node? #151

Closed
ambercool99 opened this issue Jul 28, 2023 · 21 comments
Closed

Does BDSA-2023-1613 (CVE-2023-3420) impacts Node? #151

ambercool99 opened this issue Jul 28, 2023 · 21 comments
Labels
question Further information is requested

Comments

@ambercool99
Copy link

Details

In our Node project we currently use 18..16.1 version, our security scans reported vulnerability https://nvd.nist.gov/vuln/detail/CVE-2023-3420
with V8.
the vulnerability details says that it impacts Chrome and Dabian_Linux.
Note sure if this impacts Node runtime.
I could not find anything which related this vulnerability to Node.
Please help...

Node.js version

18.16.1

Example code

No response

Operating system

Alpine (Linux)

Scope

runtime

Module and version

V8

@gireeshpunathil
Copy link
Member

@nodejs/v8 - pls advise!

@preveen-stack preveen-stack added the question Further information is requested label Aug 2, 2023
@preveen-stack
Copy link

cc @nodejs/security-release

@RafaelGSS RafaelGSS transferred this issue from nodejs/help Aug 24, 2023
@RafaelGSS
Copy link
Member

The CVE details don't say much. https://bugs.chromium.org/p/chromium/issues/detail?id=1452137 isn't public. I will wait for v8 advise.

@mhdawson
Copy link
Member

@targos do you know who a good v8 contact might be to follow up with?

@camillobruni
Copy link

V8-dev here: which V8 version are you using?

@camillobruni
Copy link

This issue applies to node as well.
The disclosed version numbers by the NVD apply as well, up to Chrome 114.0.5735.198 (v8 11.4.183.25).

The issue was related to V8's optimizing JIT compiler (Turbofan) and could be triggered by an attacker able to execute arbitrary JavaScript code.

@mcollina
Copy link
Member

@camillobruni that was my assessment, thanks for confirming my initial assessment by looking at the CVE!

This is outside our threat model.

Having said that, I think we should aim to backport a fix anyway.

@mhdawson
Copy link
Member

To clarify what @camillobruni and @mcollina said in the above comments. My understanding is that while the issue exists in the Node.js code base, it is not considered a vulnerability in Node.js because protecting againts how it can be used is outside of the Node.js threat model. This is because based on the threat model we trust the code that Node.js has been asked to run.

@preveen-stack
Copy link

Ref: https://en.m.wikipedia.org/wiki/Threat_model

@RafaelGSS
Copy link
Member

RafaelGSS commented Oct 7, 2023

@camillobruni I'm looking at it over the weekend, was it backported to v10.x? Otherwise, I believe we won't be able to add that patch to Node.js 18, unless we are sure it doesn't mix semver-major features (v11.x).

@targos
Copy link
Member

targos commented Oct 7, 2023

I opened nodejs/node#50077 with the fixes that V8 applied to their 11.4 branch.

@RafaelGSS There is no concept of v10.x or v11.x in V8. The project doesn't follow semver.

targos added a commit to nodejs/node that referenced this issue Nov 21, 2023
Original commit message:

    Merged: [runtime] Set instance prototypes directly on maps

    Bug: chromium:1452137
    (cherry picked from commit c7c447735f762f6d6d0878e229371797845ef4ab)

    Change-Id: I611c41f942e2e51f3c4b4f1d119c18410617188e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4637888
    Commit-Queue: Igor Sheludko <ishell@chromium.org>
    Auto-Submit: Igor Sheludko <ishell@chromium.org>
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/branch-heads/11.4@{#47}
    Cr-Branched-From: 8a8a1e7086dacc426965d3875914efa66663c431-refs/heads/11.4.183@{#1}
    Cr-Branched-From: 5483d8e816e0bbce865cbbc3fa0ab357e6330bab-refs/heads/main@{#87241}

Refs: v8/v8@a1efa53
PR-URL: #50077
Refs: nodejs/nodejs-dependency-vuln-assessments#151
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
targos added a commit to nodejs/node that referenced this issue Nov 21, 2023
Original commit message:

    Merged: [compiler] StackCheck can have side effects

    Bug: chromium:1452137
    (cherry picked from commit e548943e473b020fdc1de6e5543ca31b24d8b7f9)

    Change-Id: Ibd7c9b02efd12341b452e4c34a635a58a817649f
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4637129
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
    Auto-Submit: Tobias Tebbi <tebbi@chromium.org>
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/branch-heads/11.4@{#49}
    Cr-Branched-From: 8a8a1e7086dacc426965d3875914efa66663c431-refs/heads/11.4.183@{#1}
    Cr-Branched-From: 5483d8e816e0bbce865cbbc3fa0ab357e6330bab-refs/heads/main@{#87241}

Refs: v8/v8@840650f
PR-URL: #50077
Refs: nodejs/nodejs-dependency-vuln-assessments#151
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@RafaelGSS
Copy link
Member

@ambercool99 nodejs/node#50077 was merged into v20.x-staging and will be released tomorrow.

@mhdawson
Copy link
Member

@RafaelGSS can this be closed now?

@mhdawson
Copy link
Member

I'm going to close this on the assumption it has been resolved due to the comment above.

@richardlau
Copy link
Member

@mhdawson to my knowledge this was not patched in Node.js 18.

@RafaelGSS
Copy link
Member

It was merged on Node.js 20.x-staging, but wasn't released yet.

@richardlau
Copy link
Member

It was merged on Node.js 20.x-staging, but wasn't released yet.

For Node.js 20 nodejs/node#50077 went out in Node.js 20.10.0.

@RafaelGSS
Copy link
Member

Oh right, I didn't remember the backport.

@richardlau
Copy link
Member

For Node.js 18, the v20.x backport PR does not cherry-pick cleanly onto v18.x-staging so we'd need a specific v18.x backport.

targos added a commit to nodejs/node that referenced this issue Mar 20, 2024
Original commit message:

    Merged: [runtime] Set instance prototypes directly on maps

    Bug: chromium:1452137
    (cherry picked from commit c7c447735f762f6d6d0878e229371797845ef4ab)

    Change-Id: I611c41f942e2e51f3c4b4f1d119c18410617188e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4637888
    Commit-Queue: Igor Sheludko <ishell@chromium.org>
    Auto-Submit: Igor Sheludko <ishell@chromium.org>
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/branch-heads/11.4@{#47}
    Cr-Branched-From: 8a8a1e7086dacc426965d3875914efa66663c431-refs/heads/11.4.183@{#1}
    Cr-Branched-From: 5483d8e816e0bbce865cbbc3fa0ab357e6330bab-refs/heads/main@{#87241}

Refs: v8/v8@a1efa53
PR-URL: #50077
Refs: nodejs/nodejs-dependency-vuln-assessments#151
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
targos added a commit to nodejs/node that referenced this issue Mar 20, 2024
Original commit message:

    Merged: [compiler] StackCheck can have side effects

    Bug: chromium:1452137
    (cherry picked from commit e548943e473b020fdc1de6e5543ca31b24d8b7f9)

    Change-Id: Ibd7c9b02efd12341b452e4c34a635a58a817649f
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4637129
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
    Auto-Submit: Tobias Tebbi <tebbi@chromium.org>
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/branch-heads/11.4@{#49}
    Cr-Branched-From: 8a8a1e7086dacc426965d3875914efa66663c431-refs/heads/11.4.183@{#1}
    Cr-Branched-From: 5483d8e816e0bbce865cbbc3fa0ab357e6330bab-refs/heads/main@{#87241}

Refs: v8/v8@840650f
PR-URL: #50077
Refs: nodejs/nodejs-dependency-vuln-assessments#151
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@RafaelGSS
Copy link
Member

@targos I was trying to backport it to v18.x but, I think git node v8 backport has a bug?

node git:(backport-v8-to-v18) ✗ git node v8 backport f7d000a7ae7b731805338338eb51a81fbcfe2628
✔ Update local V8 cloneV8 commit backport
  ✔ Get current V8 version
  ✔ Generate patches
  ❯ Apply and commit patches to deps/v8
    ❯ Commit f7d000a7ae7b
      ⠴ Apply patch
      ◼ Increment embedder version number
      ◼ Commit patch

? Resolve merge conflicts and enter 'RESOLVED' ›

It doesn't show any conflict to resolve

node git:(backport-v8-to-v18) git status
On branch backport-v8-to-v18
nothing to commit, working tree clean

Am I using it wrong?

@targos
Copy link
Member

targos commented Jun 9, 2024

It happens when the diff doesn't apply at all (because code has changed too much between versions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

9 participants