Fix use-after free + Fix building on Windows + prebuild for Node, Electron or on x86 + add debug build + fix the tests and ci #444

aminya · 2021-04-06T14:57:05Z

This lets CMake use the best generator for that system, which increases the portability of the library and removes the need for workarounds in different environments. This change allows building with the latest Visual Studio and also on other architectures.
updated the CI to use a Node version that most people use instead of the old Node 10. This allows for realistic testing of the library. Electron uses Node 12 exclusively.
fixed the unit tests and Mocha
updated the prebuild scripts to provide prebuilds for both Electron and Node and both x64 and x86
updated old prebuildify and none-found node-addon-api
added a debug build that can be done by node-gyp rebuild --debug which has C++ exception support. This allows seeing what is going on on the C++ side.
added address sanitizer for MSVC

fixed a "use after free" that was caused because the old node-addon-api library was being used instead of the one in node_modules. This was detected by MSVC in socket construction tests

Details

  socket construction
    with child constructor
      √ should create socket with given context
      √ should set option
      √ should throw with invalid type
      √ should throw error on file descriptor limit (185ms)
      √ should throw if called as function
      √ should create socket with default context
      √ should throw with invalid option value
=================================================================
==6292==ERROR: AddressSanitizer: heap-use-after-free on address 0x142593b7efc0 at pc 0x7ffeba5ad3f1 bp 0x00f31a3df450 sp 0x00f31a3df458
READ of size 8 at 0x142593b7efc0 thread T0
    #0 0x7ffeba5ad3f0  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x18001d3f0)
    #1 0x7ff7ecb6a6f5  (C:\Program Files\nodejs\node.exe+0x14024a6f5)
    #2 0x7ff7ecba9a64  (C:\Program Files\nodejs\node.exe+0x140289a64)
    #3 0x7ff7ecb93498  (C:\Program Files\nodejs\node.exe+0x140273498)
    #4 0x7ff7ecbf87d7  (C:\Program Files\nodejs\node.exe+0x1402d87d7)
    #5 0x7ff7ecc033a7  (C:\Program Files\nodejs\node.exe+0x1402e33a7)
    #6 0x7ff7ecbd24e3  (C:\Program Files\nodejs\node.exe+0x1402b24e3)
    #7 0x7ff7ecaf8286  (C:\Program Files\nodejs\node.exe+0x1401d8286)
    #8 0x7ff7ecb7224a  (C:\Program Files\nodejs\node.exe+0x14025224a)
    #9 0x7ff7ec99822b  (C:\Program Files\nodejs\node.exe+0x14007822b)
    #10 0x7ff7ed9e016b  (C:\Program Files\nodejs\node.exe+0x1410c016b)
    #11 0x7ffefc0b7033  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017033)
    #12 0x7ffefd622650  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x180052650)

0x142593b7efc0 is located 0 bytes inside of 344-byte region [0x142593b7efc0,0x142593b7f118)
freed by thread T0 here:
    #0 0x7ffe509dceed  (C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\VC\Tools\MSVC\14.29.29917\bin\Hostx64\x64\clang_rt.asan_dbg_dynamic-x86_64.dll+0x18005ceed)
    #1 0x7ffeba70b06a  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x18017b06a)
    #2 0x7ffef5c4102f  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x18000102f)
    #3 0x7ffef5c441d6  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x1800041d6)
    #4 0x7ffef5c4659a  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x18000659a)
    #5 0x7ffef5c42cd1  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x180002cd1)
    #6 0x7ffef5c46cea  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x180006cea)
    #7 0x7ffefd671fee  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800a1fee)
    #8 0x7ffefd600938  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x180030938)
    #9 0x7ffef5c4696e  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x18000696e)
    #10 0x7ffef5c41c1d  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x180001c1d)
    #11 0x7ffef5c4218a  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x18000218a)
    #12 0x7ffef5c42ec4  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x180002ec4)
    #13 0x7ffef5c46cea  (C:\WINDOWS\SYSTEM32\VCRUNTIME140_1D.dll+0x180006cea)
    #14 0x7ffefd671f6e  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800a1f6e)
    #15 0x7ffefd621453  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x180051453)
    #16 0x7ffefd670a9d  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800a0a9d)
    #17 0x7ffefb394b58  (C:\WINDOWS\System32\KERNELBASE.dll+0x180034b58)
    #18 0x7ffee769b45f  (C:\WINDOWS\SYSTEM32\VCRUNTIME140D.dll+0x18000b45f)
    #19 0x7ffeba595d43  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x180005d43)
    #20 0x7ffeba595b78  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x180005b78)
    #21 0x7ffeba595a7e  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x180005a7e)
    #22 0x7ffeba704a54  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x180174a54)
    #23 0x7ffeba703fe6  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x180173fe6)
    #24 0x7ffeba6341c2  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x1800a41c2)
    #25 0x7ffeba637727  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x1800a7727)
    #26 0x7ffeba633f37  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x1800a3f37)
    #27 0x7ff7ecb7b9c5  (C:\Program Files\nodejs\node.exe+0x14025b9c5)
    #28 0x7ff7ed3f23ce  (C:\Program Files\nodejs\node.exe+0x140ad23ce)

previously allocated by thread T0 here:
    #0 0x7ffe509dc2f2  (C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\VC\Tools\MSVC\14.29.29917\bin\Hostx64\x64\clang_rt.asan_dbg_dynamic-x86_64.dll+0x18005c2f2)
    #1 0x7ffeba63419d  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x1800a419d)
    #2 0x7ffeba637727  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x1800a7727)
    #3 0x7ffeba633f37  (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x1800a3f37)
    #4 0x7ff7ecb7b9c5  (C:\Program Files\nodejs\node.exe+0x14025b9c5)
    #5 0x7ff7ed3f23ce  (C:\Program Files\nodejs\node.exe+0x140ad23ce)
    #6 0x7ff7ed3f15eb  (C:\Program Files\nodejs\node.exe+0x140ad15eb)
    #7 0x7ff7ed3f1c3f  (C:\Program Files\nodejs\node.exe+0x140ad1c3f)
    #8 0x7ff7ed3f1a92  (C:\Program Files\nodejs\node.exe+0x140ad1a92)
    #9 0x7ff7ed4cd7fc  (C:\Program Files\nodejs\node.exe+0x140bad7fc)
    #10 0x7ff7ed463020  (C:\Program Files\nodejs\node.exe+0x140b43020)
    #11 0x7ff7ed5586c0  (C:\Program Files\nodejs\node.exe+0x140c386c0)
    #12 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #13 0x7ff7ed462f29  (C:\Program Files\nodejs\node.exe+0x140b42f29)
    #14 0x7ff7ed5586c0  (C:\Program Files\nodejs\node.exe+0x140c386c0)
    #15 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #16 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #17 0x7ff7ed45e858  (C:\Program Files\nodejs\node.exe+0x140b3e858)
    #18 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #19 0x7ff7ed45e858  (C:\Program Files\nodejs\node.exe+0x140b3e858)
    #20 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #21 0x7ff7ed45e858  (C:\Program Files\nodejs\node.exe+0x140b3e858)
    #22 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #23 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #24 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #25 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #26 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #27 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)
    #28 0x7ff7ed466081  (C:\Program Files\nodejs\node.exe+0x140b46081)

SUMMARY: AddressSanitizer: heap-use-after-free (\\?\C:\Users\aminy\Documents\GitHub\zeromq.js\build\Debug\zeromq.node+0x18001d3f0)
Shadow bytes around the buggy address:
  0x06844606fda0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x06844606fdb0: fd fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa
  0x06844606fdc0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x06844606fdd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x06844606fde0: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa
=>0x06844606fdf0: fa fa fa fa fa fa fa fa[fd]fd fd fd fd fd fd fd
  0x06844606fe00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x06844606fe10: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x06844606fe20: fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x06844606fe30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x06844606fe40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc

</details>

Fixes #443
Fixes #432
Fixes #419
Fixes #396

aminya

The TypeScript type compatibility tests are unnecessary. Zermoq has already jumped to v6.

flowck · 2021-04-09T18:54:57Z

@aminya Should the testes around Typescript compatibility be removed since they are not needed anymore?

aminya · 2021-04-09T19:49:35Z

@aminya Should the testes around Typescript compatibility be removed since they are not needed anymore?

Yes, I would remove them. Even TypeScript doesn't maintain type stability, and that's why some of the tests are failing.

flowck · 2021-04-09T20:17:41Z

@aminya Do yo who is the lead maintainer of this project?

@rolftimmermans @rgbkrk Can you guys help?

Thank you.

aminya · 2021-04-09T21:04:39Z

The core library which Zeromq.js uses is still active. I updated the library to 4.3.4 in this PR. So, maybe someone from the repository can take a look at this.
https://github.com/zeromq/libzmq
cc: @bluca

rolftimmermans · 2021-04-12T07:58:15Z

There seem to be a lot of CI failures. Did anyone check if they are related to this change or not?

aminya · 2021-04-12T08:04:14Z

There seem to be a lot of CI failures. Did anyone check if they are related to this change or not?

Yes, it is related to TypeScript type compatibility which happens on old TypeScript versions. See the above comments. We don't actually need these tests because they are caused by TypeScript and not us. Zeromq has already bumped up the major version, so trying to fulfill these technical debts isn't healthy for the life of this project.

lierdakil · 2021-04-13T15:27:06Z

I'm seeing some failing tests apparently not related to TypeScript:

  1) socket with inproc close
       in gc finalizer
         should release reference to context:
      AssertionError: expected false to equal true
      + expected - actual
      -false
      +true
      
      at Context.<anonymous> (test/unit/socket-close-test.ts:156:16)
  2) socket with tcp close
       in gc finalizer
         should release reference to context:
      AssertionError: expected false to equal true
      + expected - actual
      -false
      +true
      
      at Context.<anonymous> (test/unit/socket-close-test.ts:156:16)
  3) socket with ipc close
       in gc finalizer
         should release reference to context:
      AssertionError: expected false to equal true
      + expected - actual
      -false
      +true
      
      at Context.<anonymous> (test/unit/socket-close-test.ts:156:16)

Not sure if it's related to the changes made here or not.

aminya · 2021-04-13T21:17:55Z

I'm seeing some failing tests apparently not related to TypeScript:

The CI on the master branch doesn't even start the tests! If you manage to build the library and manage to call the tests, then the same tests fail on master too.

Here is a CI on master! 🙄
https://travis-ci.org/github/zeromq/zeromq.js/jobs/765826517

aminya · 2021-04-13T22:58:20Z

Atom 1.56 was released today with Electron 9, which means we are already late as Zeromq doesn't build anymore. If @zeromq doesn't plan to release a new version, we have to create a fork.

This lets CMake use the best generator for that system, which increases the portability of the library and removing the need for workarounds in different environments

Replaces old mocha.opts with .mocharc.js. Now the mocha config is detected correctly.

Node 13+ are able to prebuild without the v8 compression error

There are no breaking changes: node-ffi-napi/weak-napi@v1.0.3...v2.0.2

It is not the responsibility of Zeromq to be buildable on old TypeScript or fullfil technical debts that TypeScript doesn't comply to.

aminya · 2021-04-15T10:56:19Z

I got access to this repository. I plan to merge this and then try to debug the memory leak issues (which seems to cause the GC tests to fail as well).

Organized the commits. The related commits are now together, and their messages are more descriptive.

captainsafia · 2021-04-15T16:08:46Z

.travis.yml

@@ -1,59 +1,62 @@
 language: node_js


Can we favor GitHub Actions over Travis for the 6.x branch moving forward?

Yes, we can. I prefer to do this in the follow-up pull requests to keep things simpler.

test/unit/typings-compatibility-test.ts

binding.gyp

aminya changed the title ~~Fix building with Visual Studio 2019~~ Fix building using any Visual Studio on Windows Apr 6, 2021

aminya force-pushed the fix-windows-build branch from b15cd83 to 9ae0120 Compare April 6, 2021 15:08

This was referenced Apr 6, 2021

ZeroMq doesn't build for Electron 9 n-riesco/jmp#31

Closed

Zeromq doesn't build on Electron 9 nteract/hydrogen#2034

Closed

aminya changed the title ~~Fix building using any Visual Studio on Windows~~ Fix building on Windows + prebuild for Node and Electron and x86 + fix the tests and ci Apr 6, 2021

aminya commented Apr 8, 2021

View reviewed changes

This was referenced Apr 8, 2021

Status of project #442

Closed

Subscriber Memory Leak #390

Closed

aminya changed the title ~~Fix building on Windows + prebuild for Node and Electron and x86 + fix the tests and ci~~ Fix building on Windows + prebuild for Node, Electron or on x86 + add debug build + fix the tests and ci Apr 8, 2021

aminya force-pushed the fix-windows-build branch 2 times, most recently from 4deb9c7 to 2025aa0 Compare April 8, 2021 07:34

aminya changed the title ~~Fix building on Windows + prebuild for Node, Electron or on x86 + add debug build + fix the tests and ci~~ Fix use-after free + Fix building on Windows + prebuild for Node, Electron or on x86 + add debug build + fix the tests and ci Apr 8, 2021

aminya mentioned this pull request Apr 8, 2021

Access violation in TypedArrayOf nodejs/node-addon-api#965

Closed

aminya force-pushed the fix-windows-build branch 2 times, most recently from 5cb664a to 2601d4a Compare April 14, 2021 00:08

aminya mentioned this pull request Apr 14, 2021

Build zeroMq v5 on Electron 9 - Windows #445

Merged

aminya added 2 commits April 15, 2021 05:37

fix: let Cmake choose the generator

88e553e

This lets CMake use the best generator for that system, which increases the portability of the library and removing the need for workarounds in different environments

fix: fix building with VS2019 and VS2017

3c61fea

aminya force-pushed the fix-windows-build branch from 2601d4a to c6a2544 Compare April 15, 2021 10:39

aminya added 2 commits April 15, 2021 05:45

test: fix mocha config

22dee8f

Replaces old mocha.opts with .mocharc.js. Now the mocha config is detected correctly.

fix: resolve always expects a value - pass undefined

0c61aa0

aminya added 14 commits April 15, 2021 05:45

fix: remove none-existing node-addon-api from package.json

22d878a

fix: add the required libs (es2018 and dom) to tsconfig

b8d295d

fix: update libzmq version

18307a5

ci: use Node 14 in Travis

c7090c9

Node 13+ are able to prebuild without the v8 compression error

ci: use python 3 in Travis

520f476

chore: prevent minwindef.h from defining max macro in the debug build

629dfad

chore: add BuildType env variable to allow debug builds

9c79db2

chore: enable exceptions in debug builds

0e0acbb

chore: add address sanitizer support for MSVC in the debug build

dc17ddd

fix: update prebuildify

2fb0d1e

fix: prebuildify for both Node and Electron

47eaf2f

fix: include node-addon-api from the dependency and remove vendor folder

608a615

test: update weak-napi to help with gc detection

c858506

There are no breaking changes: node-ffi-napi/weak-napi@v1.0.3...v2.0.2

test: disable typeScript-compatibility for older than 3.4.x

be3eff2

It is not the responsibility of Zeromq to be buildable on old TypeScript or fullfil technical debts that TypeScript doesn't comply to.

aminya force-pushed the fix-windows-build branch from c6a2544 to be3eff2 Compare April 15, 2021 10:53

aminya requested a review from captainsafia April 15, 2021 10:56

aminya added 6.x bug build enhancement labels Apr 15, 2021

captainsafia reviewed Apr 15, 2021

View reviewed changes

chore: parametrize sanitizer build

740b4df

aminya merged commit 813a15a into zeromq:master Apr 18, 2021

aminya deleted the fix-windows-build branch April 18, 2021 14:03

naiba4 mentioned this pull request Sep 6, 2024

[Snyk] Upgrade: bs58check, dotenv, lru-cache, markdown-it, zeromq naiba4/btc-rpc-explorer#3

Open

This was referenced Sep 13, 2024

[Snyk] Upgrade: bs58check, dotenv, lru-cache, markdown-it, zeromq naiba4/btc-rpc-explorer#4

Open

[Snyk] Upgrade: bs58check, dotenv, lru-cache, markdown-it, zeromq naiba4/btc-rpc-explorer#5

Open

naiba4 mentioned this pull request Sep 23, 2024

[Snyk] Upgrade zeromq from 5.3.1 to 6.0.4 naiba4/btc-rpc-explorer#7

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix use-after free + Fix building on Windows + prebuild for Node, Electron or on x86 + add debug build + fix the tests and ci #444

Fix use-after free + Fix building on Windows + prebuild for Node, Electron or on x86 + add debug build + fix the tests and ci #444

aminya commented Apr 6, 2021 •

edited

Loading

aminya left a comment

flowck commented Apr 9, 2021

aminya commented Apr 9, 2021 •

edited

Loading

flowck commented Apr 9, 2021

aminya commented Apr 9, 2021 •

edited

Loading

rolftimmermans commented Apr 12, 2021

aminya commented Apr 12, 2021 •

edited

Loading

lierdakil commented Apr 13, 2021

aminya commented Apr 13, 2021 •

edited

Loading

aminya commented Apr 13, 2021

aminya commented Apr 15, 2021 •

edited

Loading

captainsafia Apr 15, 2021

aminya Apr 16, 2021

Fix use-after free + Fix building on Windows + prebuild for Node, Electron or on x86 + add debug build + fix the tests and ci #444

Fix use-after free + Fix building on Windows + prebuild for Node, Electron or on x86 + add debug build + fix the tests and ci #444

Conversation

aminya commented Apr 6, 2021 • edited Loading

aminya left a comment

Choose a reason for hiding this comment

flowck commented Apr 9, 2021

aminya commented Apr 9, 2021 • edited Loading

flowck commented Apr 9, 2021

aminya commented Apr 9, 2021 • edited Loading

rolftimmermans commented Apr 12, 2021

aminya commented Apr 12, 2021 • edited Loading

lierdakil commented Apr 13, 2021

aminya commented Apr 13, 2021 • edited Loading

aminya commented Apr 13, 2021

aminya commented Apr 15, 2021 • edited Loading

captainsafia Apr 15, 2021

Choose a reason for hiding this comment

aminya Apr 16, 2021

Choose a reason for hiding this comment

aminya commented Apr 6, 2021 •

edited

Loading

aminya commented Apr 9, 2021 •

edited

Loading

aminya commented Apr 9, 2021 •

edited

Loading

aminya commented Apr 12, 2021 •

edited

Loading

aminya commented Apr 13, 2021 •

edited

Loading

aminya commented Apr 15, 2021 •

edited

Loading