-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
Description
Instead, anything in crypto
module accepting JS strings should verify that the strings are well-formed before converting them to Buffers, and throw
In before: the argument that those strings would be treated as equal in other contexts is not valid
E.g., using bn.js@4
:
import BN from 'bn.js' // 4
import { hash } from 'node:crypto'
const [a, b] = JSON.parse(payload) // user input
console.log('Hashes equal?', hash('sha256', a) === hash('sha256', b))
console.log('Strings equal?', a === b)
console.log('bn.js values:', (new BN(a)).toNumber(), (new BN(b)).toNumber())
Output:
Hashes equal? true
Strings equal? false
bn.js values: 55229 57036
If anything anywhere uses string hashes for any purpose like k
/nonce
calculation (ref: GHSA-vjh7-7g9h-fjfh), e.g. in curve cryptography or in a block/stream cipher, this can lead to private key/data exposure
Also obviously it allows to do things like this:
import { hash } from 'node:crypto'
const [a, b, c, d] = JSON.parse(payload) // user input
const sha256 = (x) => hash('sha256', x)
console.log('a === b?', sha256(a) === sha256(b))
console.log('c === d?', sha256(c) === sha256(d))
console.log('a + c === b + d?', sha256(a + c) === sha256(b + d))
a === b? true
c === d? true
a + c === b + d? false
As a solution, enforce usage of String.prototype.isWellFormed
on all string input and throw
Obviously, also applicable to sign/verify and other APIs:
import { generateKeyPairSync, createSign, createVerify } from 'node:crypto'
const { privateKey, publicKey } = generateKeyPairSync('rsa', { modulusLength: 2048 })
const [a, b] = JSON.parse(payload) // user input
const sign = createSign('SHA256')
sign.update(a)
sign.end()
const signature = sign.sign(privateKey)
const verify = createVerify('SHA256')
verify.update(b)
verify.end()
console.log('types:', typeof a, typeof b)
console.log('verified?', verify.verify(publicKey, signature))
console.log('a === b?', a === b)
types: string string
verified? true
a === b? false
WebCrypto is not affected as it doesn't accept strings
Node.js accepts JS strings but operates on byte arrays, and js-string-to-byte-array transform is not injective
There should be a safeguard against transform collisions