Skip to content

Update Padding from 0x1 to 0x6 as in SHA3 standard#313

Open
zeruniverse wants to merge 1 commit into
brix:developfrom
zeruniverse:patch-1
Open

Update Padding from 0x1 to 0x6 as in SHA3 standard#313
zeruniverse wants to merge 1 commit into
brix:developfrom
zeruniverse:patch-1

Conversation

@zeruniverse

Copy link
Copy Markdown

I observed CryptoJS SHA3 is different from PHP hash("sha3-512"). After comparing to code here: https://github.com/emn178/js-sha3 I found changing the 0x1 to 0x6 will make CryptoJS matching the standard SHA3 (at least for sha3-512)

zeruniverse added a commit to zeruniverse/Password-Manager that referenced this pull request Aug 3, 2020
@alanturing881

Copy link
Copy Markdown

Independently reproduced this while auditing crypto-js — confirming and extending this fix with broader test coverage.

Verification

Differential-tested CryptoJS.SHA3() against Node.js's native crypto.createHash('sha3-*') across all four configured output sizes (224/256/384/512), at 31 lengths spanning each size's Keccak rate boundary (72/104/136/144 bytes respectively), including the empty string:

  • Before this fix (current 0x1 padding byte): 0/31 matches for every output size (SHA3-224, SHA3-256, SHA3-384, SHA3-512) — CryptoJS.SHA3('') never matches the standard SHA-3 digest of the empty string, or any other input.
  • With this PR's one-line fix (0x10x6): 14/14 matches exactly against Node's native sha3-512 in a follow-up sweep.

This confirms the domain-separation padding byte is the sole root cause, and that the fix is correct not just for SHA3-512 (as originally reported) but for all four SHA-3 output sizes this library exposes via outputLength.

Impact note

To be clear about severity: this isn't a cryptanalytic weakness (Keccak with 0x01 padding isn't known to be weaker than FIPS 202 SHA-3 with 0x06 padding — both share the same underlying permutation and security margins). The practical impact is interoperability: CryptoJS.SHA3() currently computes the pre-standardization Keccak submission, not NIST-standardized SHA-3, so it silently produces digests that don't match any other SHA-3 implementation (OpenSSL, Node's native crypto, Python's hashlib, etc.) or FIPS-140/202 compliance requirements — for every single input.

This PR has been open and mergeable: true / mergeable_state: clean since 2020. Given it's a single-character, fully backward-compatible-with-the-standard fix that resolves a real conformance defect affecting every CryptoJS.SHA3() call, it'd be great to see this merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants