nodejs / nodejs/node

Equal hashes on non-equal JS strings are dangerous to the ecosystem

Offen
#60,267 4 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

security
Vorherrschende Sprache
JavaScript
Sterne
122k
Forks
37.3k
Ø Merge
4 T. 2 Std.
Gemergte PRs (30 T.)
283

Beschreibung

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

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginne damit, die Einstiegspunkte des Crypto-Moduls zu inventarisieren, die JavaScript-Strings akzeptieren, einschließlich hash, sign und verify, und verfolge ihre Umwandlung von Strings in Byte-Arrays. Vergleiche die Beispiele mit dem Verhalten von String.prototype.isWellFormed; die Aufgabe ist erfüllt, wenn für die relevanten APIs eine definierte Schutzmaßnahme vorhanden ist und Tests zeigen, dass fehlerhafte Strings nicht als gleichwertig behandelt werden.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
javascript, node.js
Bereich
backend, cryptography, security
Issue-Typ
Bug
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Ruhig
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
35/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.