Repository navigation
crypto.pseudoRandomBytes results in a segmentation fault #38090
Description
Activity
- addedcryptoIssues and PRs related to the crypto subsystem.Issues and PRs related to the crypto subsystem.
on Apr 5, 2021 According to the docs
size The number of bytes to generate. The size must not be larger than 2**31 - 1.
It seems we have know it will not work in
2 ** 31(2147483648). And there is an validation here but it did not work.The issue also occurs when execute in node
Welcome to Node.js v14.16.0. Type ".help" for more information. > crypto.randomBytes(2147483648) [1] 83064 segmentation fault nodeDoesn't reproduce on Node.js v15.x, probably fixed by #35093.
Can repro on Windows 10 as well.
(5628.5a40): Access violation - code c0000005 (first chance) First chance exceptions are reported before any exception handling. This exception may be expected and handled. node!v8::internal::compiler::RepresentationChanger::Uint32OverflowOperatorFor+0x42699: 00007ff7`01593079 f3aa rep stos byte ptr [rdi] 0:000> !load msec 0:000> !exploitable !exploitable 1.6.0.0 Exploitability Classification: EXPLOITABLE Recommended Bug Title: Exploitable - User Mode Write AV starting at node!v8::internal::compiler::RepresentationChanger::Uint32OverflowOperatorFor+0x0000000000042699 (Hash=0xccd97b0f.0xbe9a6a99) User mode write access violations that are not near NULL are exploitable.I'm not a JavaScript developer. I am just working on some research to detect bugs and NodeJS is one of my test targets.
Thus maybe some of my issues do not make sense, and thanks for your patient reply.
I am working on the LTS versions now. I have a personal question that whether issues that occurred on the old versions like this one are meaningful for you.
Sinceconfirmed-bugis important for me. If there is no sense of these problems, then maybe I should save my machine time to focus on more meaningful places.- addedconfirmed-bugIssues and PRs for confirmed bugs.Issues and PRs for confirmed bugs.
on Apr 5, 2021 @zyscoder First of all, thanks for all the reports you sent! Bug reports on LTS versions are certainly valid, Node.js as project would accept PR fixing that kind of crash for all supported versions.
One of the drawbacks of using an LTS version to search for bugs is you might end up discovering bugs already fixed on master: we have a "maturation" policy before backporting commits to LTS releases, so most of the times such patch will end up automatically in the LTS release line a few month later. This particular bug was fixed on a semver-major commit, so it won't be backported to v14.x line until someone takes the time to open a backport PR for it, which could never happen if no one volunteers.TL;DR, you might want to target the master branch (or maybe v16.0.0-rc.1) to decrease the probability of finding already reported bugs.
After digging into the code, I found there is a discrepancy of validation in JS side and C++ side. So, technically, the issue "exists" in master branch but it guarded by C++ backend (introduced in #35093). I would like to open a PR to address it.
@Ayase-252 why does it matter if it's handled by C++ or JS code?
@aduh95
TL;DL; It seems thebuffer.constants.MAX_LENGTHincreased fromMAX_INT(2** 31 - 1) to2 ** 32somehow in 64 bits platform(Refs: #38093). It breaks the validation in JS side. However, it has an extra guard in C++ in v15, but not in v14.Both v14 and v15 has validation for
sizeargument, asnode/lib/internal/crypto/random.js
Line 92 in 4f387c2
size = assertSize(size, 1, 0, Infinity); Let's see
assertSizenode/lib/internal/crypto/random.js
Lines 75 to 89 in 4f387c2
function assertSize(size, elementSize, offset, length) { validateNumber(size, 'size'); size *= elementSize; if (NumberIsNaN(size) || size > kMaxPossibleLength || size < 0) { throw new ERR_OUT_OF_RANGE('size', `>= 0 && <= ${kMaxPossibleLength}`, size); } if (size + offset > length) { throw new ERR_OUT_OF_RANGE('size + offset', `<= ${length}`, size + offset); } return size >>> 0; // Convert to uint32. } Note that
kMaxPossibleLengthisnode/lib/internal/crypto/random.js
Line 61 in 4f387c2
const kMaxPossibleLength = MathMin(kMaxLength, kMaxUint32); where
kMaxLengthisbuffer.kMaxLength(alias ofbuffer.constants.MAX_LENGTH).Since
buffer.kMaxLengthraised to2 ** 32, what it actually doing is ensures that whether size is less than2 ** 32 - 1, not2 ** 31 - 1as expected.However, in v15, an extra guard is introduced as part of #35093. It ensures
sizemust be less than2 ** 31 - 1and preventsabortwhensizeis larger than2 ** 31.node/src/crypto/crypto_random.cc
Lines 54 to 57 in 4f387c2
if (UNLIKELY(size > INT_MAX)) { THROW_ERR_OUT_OF_RANGE(env, "buffer is too large"); return Nothing<bool>(); } But v14 is out of luck to have a C++ guard, therefore aborts when passing
2 ** 31.Reacted by silverwind- added a commit that references this issue
on Apr 6, 2021 - added a commit that references this issue
on Apr 12, 2021 - added a commit that references this issue
on Apr 15, 2021 - added a commit that references this issue
on May 1, 2021 - added a commit that references this issue
on May 8, 2021 Closing as #38096 has been backported to v14.x, it should be fixed.
I think this is the same issue, but it's happening in Node 16.1.0
When I run:
node --experimental-repl-awaitand paste:const UDEF = typeof undefined const sha256 = async (str) => { const self = {} // Isomorphic polyfill, for node, repl, window, workers self.crypto = typeof crypto === UDEF || crypto.webcrypto ? (await import('crypto')).webcrypto : crypto self.TextEncoder = typeof TextEncoder === UDEF ? (await import('util')).TextEncoder : TextEncoder // Browser code will work with or without self. console.log(self.crypto) const buf = await self.crypto.subtle.digest("SHA-256", new self.TextEncoder("utf-8").encode(str)) return Array.prototype.map.call(new Uint8Array(buf), x=>(('00'+x.toString(16)).slice(-2))).join('') } await sha256('Hello Mars')in a project with an "imports" to a non existing folder
I get a segmentation fault core dump
@rayfoss I'm not able to reproduce on v16.1.0 nor on master (macOS 11.3.1). If you are positive there's another bug, maybe it'd be worth opening a new issue.
@aduh95 Tracked it down to having an imports to a folder that doesn't exist... not sure why that last line trigger it, but making the last line an assignment doesn't. Regardless, thats not a useful error.
What steps will reproduce the bug?
Setup a node instance,
and run the following javascript code.
Then a segmentation fault occurs.
How often does it reproduce? Is there a required condition?
This problem can always be triggered following the steps above.
Noticed that
2147483647would not cause this problem. It seems to result from an integer overflow.What is the expected behavior?
If any error occurs, an exception or other similar error-reporting stuff should be thrown. There is no reason to abort the whole node process.
What do you see instead?
Additional information