Home Original page

node-api: type tag external values without v8::Private by legendecas · Pull Request #51149 · nodejs/node

@nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

needs-ci

PRs that need a full CI run.

node-api

Issues and PRs related to the Node-API.

labels

Dec 13, 2023

@legendecas 

v8::External can not have any properties and private properties. Type
tag v8::External with a wrapper struct without setting a private
property on the v8::External.

mhdawson

gabrielschulhof

@targos targos added the commit-queue

Add this label to land a pull request using GitHub Actions.

label

Dec 22, 2023

RafaelGSS pushed a commit that referenced this pull request

Jan 2, 2024 
v8::External can not have any properties and private properties. Type
tag v8::External with a wrapper struct without setting a private
property on the v8::External.

PR-URL: #51149
Fixes: nodejs/node-v8#273
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>

richardlau pushed a commit that referenced this pull request

Mar 25, 2024 
v8::External can not have any properties and private properties. Type
tag v8::External with a wrapper struct without setting a private
property on the v8::External.

PR-URL: #51149
Fixes: nodejs/node-v8#273
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>

Koromix added a commit to Koromix/rygel that referenced this pull request

Apr 4, 2024 
At least it seems to be a bug according to the N-API
documentation, or a documentation mistake, and probably comes
from here: nodejs/node#51149

This Node.js code change stores the napi_type_tag pointer and not
its value, but Koffi was using temporaries.

Koromix added a commit to Koromix/rygel that referenced this pull request

Apr 5, 2024 
At least it seems to be a bug according to the N-API
documentation, or a documentation mistake, and probably comes
from here: nodejs/node#51149

This Node.js code change stores the napi_type_tag pointer and not
its value, but Koffi was using temporaries.

This was referenced

Apr 6, 2024

nodejs-github-bot pushed a commit that referenced this pull request

Apr 15, 2024 
In order to adapt to V8 changes regarding storing private
properties on Externals, ExternalWrapper objects were introduced
in #51149.

However, this new code stores the type tag pointer and not the
128-bit value inside. This breaks some pre-existing code that
were making temporary tags. It also means that unloading the module
will cause existing External objects to have a tag pointer that
points nowhere (use-after-free bug).

Change ExternalWrapper to store tags by value to fix this regression.

PR-URL: #52426
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>

thisalihassan pushed a commit to thisalihassan/node that referenced this pull request

Apr 15, 2024 
In order to adapt to V8 changes regarding storing private
properties on Externals, ExternalWrapper objects were introduced
in nodejs#51149.

However, this new code stores the type tag pointer and not the
128-bit value inside. This breaks some pre-existing code that
were making temporary tags. It also means that unloading the module
will cause existing External objects to have a tag pointer that
points nowhere (use-after-free bug).

Change ExternalWrapper to store tags by value to fix this regression.

PR-URL: nodejs#52426
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>

aduh95 pushed a commit that referenced this pull request

Apr 29, 2024 
In order to adapt to V8 changes regarding storing private
properties on Externals, ExternalWrapper objects were introduced
in #51149.

However, this new code stores the type tag pointer and not the
128-bit value inside. This breaks some pre-existing code that
were making temporary tags. It also means that unloading the module
will cause existing External objects to have a tag pointer that
points nowhere (use-after-free bug).

Change ExternalWrapper to store tags by value to fix this regression.

PR-URL: #52426
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>

marco-ippolito pushed a commit that referenced this pull request

May 2, 2024 
In order to adapt to V8 changes regarding storing private
properties on Externals, ExternalWrapper objects were introduced
in #51149.

However, this new code stores the type tag pointer and not the
128-bit value inside. This breaks some pre-existing code that
were making temporary tags. It also means that unloading the module
will cause existing External objects to have a tag pointer that
points nowhere (use-after-free bug).

Change ExternalWrapper to store tags by value to fix this regression.

PR-URL: #52426
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>

marco-ippolito pushed a commit that referenced this pull request

May 3, 2024 
In order to adapt to V8 changes regarding storing private
properties on Externals, ExternalWrapper objects were introduced
in #51149.

However, this new code stores the type tag pointer and not the
128-bit value inside. This breaks some pre-existing code that
were making temporary tags. It also means that unloading the module
will cause existing External objects to have a tag pointer that
points nowhere (use-after-free bug).

Change ExternalWrapper to store tags by value to fix this regression.

PR-URL: #52426
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>