Skip to content

message: "Assertion `val->IsArrayBufferView()' failed." #41949

Closed
#42062
Closed
message: "Assertion `val->IsArrayBufferView()' failed."#41949
#42062
Labels
string_decoderIssues and PRs related to the string_decoder subsystem.

Description

@sigdevel
sigdevel
opened on Feb 12, 2022

Version

node-v16.13.2

Platform

Linux astra 4.15.3-141-generic #astra26+ci17 x86_64 GNU/Linux (debian-based)

Subsystem

No response

What steps will reproduce the bug?

\\1
var StringDecoder = require('string_decoder').StringDecoder;
var d = new StringDecoder('utf8');
var b = Buffer('abc');
console.log(b);
console.log(d.__proto__.write(b));
\\2
var StringDecoder = require('string_decoder').StringDecoder;
var d = new StringDecoder('utf8').__proto__;
var b = Buffer('abc');
console.log(b);
console.log(d.write(b));

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

Standart buildCompiled with: gcc (version 8.3.0) ;
Configure params: --prefix="/target/dir";
Builded binary info:
node: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.6.32, not stripped.

What is the expected behavior?

No response

What do you see instead?

Stderr with run crafted sample: Abort;

<Buffer 61 62 63>
./node[7121]: ../src/node_buffer.cc:246:char *node::Buffer::Data(Local<v8::Value>): Assertion `val->IsArrayBufferView()' failed.
 1: 0x10b84c7 node::DumpBacktrace(_IO_FILE*) [./node]
 2: 0x17584a1 node::Abort() [./node]
 3: 0x1756c1e  [./node]
 4: 0x15509af node::Buffer::Data(v8::Local<v8::Value>) [./node]
 5: 0x213f228  [./node]
 6: 0x2a7f008 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [./node]
 7: 0x2a7e532  [./node]
 8: 0x2a7d1ca v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [./node]
 9: 0x3cb9299  [./node]
Аварийный останов

Additional information

Call might be what is causing in type proto (the last dot).

Activity

  1. benjamingr commented on Feb 12, 2022

    @benjamingr
    benjamingr
    on Feb 12, 2022
    Member

    I'm not sure I understand what the ask here is - to fail with a catchable exception?

    Something like:

    diff --git a/lib/string_decoder.js b/lib/string_decoder.js
index 7447bb3f46..ab6bfb306b 100644
--- a/lib/string_decoder.js
+++ b/lib/string_decoder.js
@@ -42,6 +42,7 @@ const {
 } = internalBinding('string_decoder');
 const internalUtil = require('internal/util');
 const {
+  ERR_ILLEGAL_CONSTRUCTOR,
   ERR_INVALID_ARG_TYPE,
   ERR_UNKNOWN_ENCODING
 } = require('internal/errors').codes;
@@ -101,6 +102,9 @@ StringDecoder.prototype.write = function write(buf) {
     throw new ERR_INVALID_ARG_TYPE('buf',
                                    ['Buffer', 'TypedArray', 'DataView'],
                                    buf);
+  if (!this[kNativeDecoder]) {
+    throw new ERR_ILLEGAL_CONSTRUCTOR();
+  }
   return decode(this[kNativeDecoder], buf);
 };
  2. benjamingr
    added
    string_decoderIssues and PRs related to the string_decoder subsystem.
    on Feb 13, 2022

  3. benjamingr commented on Feb 13, 2022

    @benjamingr
    benjamingr
    on Feb 13, 2022
    Member

    My intuition is "we shouldn't guard against this sort of thing" most likely. @addaleax wdyt?

  4. addaleax commented on Feb 14, 2022

    @addaleax
    addaleax
    on Feb 14, 2022
    Member

    I mean, I personally think that it’s fine to add guards (in C++, not JS, to keep performance parity), but I think that ship has sailed for Node.js internals.

  5. RaisinTen
    added 2 commits that reference this issue on Feb 20, 2022

    string_decoder: fix crash when calling __proto__.write()

    c138a51

    string_decoder: fix crash when calling __proto__.write()

    abb6183

  6. RaisinTen commented on Feb 20, 2022

    @RaisinTen
    RaisinTen
    on Feb 20, 2022
    Member

    C++ fix - #42061
    JS fix - #42062
    Running benchmarks on both to be sure about the differences.

  7. sigdevel commented on Feb 22, 2022

    @sigdevel
    sigdevel
    on Feb 22, 2022
    Author

    Great, I think this issue can be closed.

  8. sigdevel
    closed this as completedon Feb 22, 2022

  9. RaisinTen commented on Feb 22, 2022

    @RaisinTen
    RaisinTen
    on Feb 22, 2022
    Member

    We can keep this issue open for now and it will get closed automatically when any of the PRs land since I have added Fixes: <issue-link> to the descriptions. :)

  10. RaisinTen
    reopened this on Feb 22, 2022

  11. sigdevel commented on Feb 22, 2022

    @sigdevel
    sigdevel
    on Feb 22, 2022
    Author

    OK Thanks!

  12. RaisinTen commented on Feb 27, 2022

    @RaisinTen
    RaisinTen
    on Feb 27, 2022
    Member

    @addaleax there doesn't appear to be any significant perf regressions in the JS fix or the C++ fix, so should we go ahead with the JS fix, given that we mostly do such validations in JS?

  13. nodejs-github-bot
    closed this as completedin #42062on Mar 8, 2022
  14. nodejs-github-bot
    added a commit that references this issue on Mar 8, 2022

    string_decoder: fix crash when calling __proto__.write()

    24fc302
  15. bengl
    added a commit that references this issue on Mar 21, 2022

    string_decoder: fix crash when calling __proto__.write()

    3492a0e
  16. danielleadams
    added 4 commits that reference this issue on Apr 21, 2022

    string_decoder: fix crash when calling __proto__.write()

    6e65f9d

    string_decoder: fix crash when calling __proto__.write()

    f21f104
  17. xtx1130
    added a commit that references this issue on Apr 25, 2022

    string_decoder: fix crash when calling __proto__.write()

    e444dbe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    string_decoderIssues and PRs related to the string_decoder subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions