Equal hashes on non-equal JS strings are dangerous to the ecosystem
Nadie ha tomado este issue todavía.
- Lenguaje dominante
- JavaScript
- Estrellas
- 122k
- Forks
- 37.3k
- Merge medio
- 4 d 2 h
- PR fusionados (30 d)
- 283
Descripción
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
Guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Línea de trabajo
Comienza inventariando los puntos de entrada del módulo de crypto que aceptan cadenas de JavaScript, incluidos hash, sign y verify, y sigue su conversión de cadenas a matrices de bytes. Compara los ejemplos con el comportamiento de String.prototype.isWellFormed; el trabajo estará completo cuando exista una salvaguarda definida en las APIs relevantes y las pruebas demuestren que las cadenas malformadas no se tratan como equivalentes.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- javascript, node.js
- Área
- backend, cryptography, security
- Tipo de issue
- Error
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Estado de actividad
- Tranquilo
- Claridad
- Bastante claro
- Aptitud para principiantes
- 35/100