wasm-voxel AES is broken — round function missing, keystream is input/IV-independent (not real AES) #811

Open
opened 2026-07-23 05:47:02 +00:00 by spikerj · 1 comment
Owner

Bug (found via coverage wave 3, spikersoft-angular PR #551). Security/interop.

libraries/game/wasm-voxel/src/_components/wasm-voxel/ts/util/aes.ts — the AES.encrypt() method's main round loop is a broken port. Instead of the AES round function (SubBytes/ShiftRows/MixColumns via the T1..T4 lookup tables applied to the running state t), it does:

for (let r = 1; r < rounds; r++) {
  for (let i = 0; i < 4; i++) { a[i] = this._Ke[r][i]; }
  t = a.slice();   // overwrites the state with the round key every round
}

The state t is unconditionally replaced by round keys each round, so after the loop it equals _Ke[rounds-1]completely independent of the input block. The T-table tables (T1..T4) are not even defined in the file. Net effect: AES.encrypt(block) returns a value that depends only on the key, not the 16-byte input.

Consequences for the exported CFB mode (ModeOfOperationCFB):

  • The keystream is fixed per key and reused for every block (the ciphertext feedback is ignored), and the IV has no effect. This is catastrophic keystream reuse — trivially breakable, and it is NOT AES.
  • It is used in production: ts/net/minecraft/client/network/NetworkManager.ts:276-277 builds aes.ModeOfOperation.cfb(secretKey, secretKey, 1) for connection encrypt/decrypt. Two instances of this same broken code round-trip with each other, so it appears to work locally, but it provides no real confidentiality and will not interoperate with a correct AES/Minecraft-protocol peer.

Verification: encrypting the same plaintext under two different IVs yields identical ciphertext (pinned as a characterization test in PR #551, aes.spec.ts, referencing this ticket).

Fix: restore a correct AES round function (port the real aes-js with its T1..T4 tables, or swap to a vetted WebCrypto/aes-js dependency). When fixed, flip the characterization assertions (different IV → different ciphertext).

**Bug (found via coverage wave 3, spikersoft-angular PR #551).** Security/interop. `libraries/game/wasm-voxel/src/_components/wasm-voxel/ts/util/aes.ts` — the `AES.encrypt()` method's main round loop is a broken port. Instead of the AES round function (SubBytes/ShiftRows/MixColumns via the T1..T4 lookup tables applied to the running state `t`), it does: ```js for (let r = 1; r < rounds; r++) { for (let i = 0; i < 4; i++) { a[i] = this._Ke[r][i]; } t = a.slice(); // overwrites the state with the round key every round } ``` The state `t` is unconditionally replaced by round keys each round, so after the loop it equals `_Ke[rounds-1]` — **completely independent of the input block**. The T-table tables (T1..T4) are not even defined in the file. Net effect: `AES.encrypt(block)` returns a value that depends only on the key, not the 16-byte input. Consequences for the exported CFB mode (`ModeOfOperationCFB`): - The keystream is **fixed per key** and **reused for every block** (the ciphertext feedback is ignored), and the **IV has no effect**. This is catastrophic keystream reuse — trivially breakable, and it is NOT AES. - It is used in production: `ts/net/minecraft/client/network/NetworkManager.ts:276-277` builds `aes.ModeOfOperation.cfb(secretKey, secretKey, 1)` for connection encrypt/decrypt. Two instances of this same broken code round-trip with each other, so it appears to work locally, but it provides no real confidentiality and will not interoperate with a correct AES/Minecraft-protocol peer. **Verification:** encrypting the same plaintext under two different IVs yields identical ciphertext (pinned as a characterization test in PR #551, `aes.spec.ts`, referencing this ticket). **Fix:** restore a correct AES round function (port the real aes-js with its T1..T4 tables, or swap to a vetted WebCrypto/`aes-js` dependency). When fixed, flip the characterization assertions (different IV → different ciphertext).
Author
Owner

Audited against origin/masterSTILL BROKEN, and the test(coverage) commit did not fix it. Of the eleven small bugs I swept this pass, this is the highest real-world severity.

The commit that references this ticket only pinned it. 577f78e8 ("test(coverage): wasm-voxel AES-CFB + SECURITY bug #811") touched exactly two files: aes.spec.ts (+97) and COVERAGE-LOOP.md. Zero source files. So anyone searching git log --grep='#811' finds a commit that reads like a fix and isn't one.

The defect is unchanged. libraries/game/wasm-voxel/src/_components/wasm-voxel/ts/util/aes.ts:137-141 — the round loop still overwrites the state with round keys (a[i] = this._Ke[r][i]; … t = a.slice();) rather than applying SubBytes/ShiftRows/MixColumns. The T1–T4 tables are still absent; only the S box exists (:47). So there is no round function, and the keystream is independent of both input and IV.

And it is live, not latent. NetworkManager.ts:276-277 still constructs it for both directions:

this.decryption = new aes.ModeOfOperation.cfb(secretKey, secretKey, 1);
this.encryption = new aes.ModeOfOperation.cfb(secretKey, secretKey, 1);

Note it also passes secretKey as the IV, so key and IV are identical — which compounds the missing round function rather than mitigating it. Whatever this protects has effectively no confidentiality.

For context from the same sweep: of the eleven tickets, #792, #808 and #840 turned out to be latent (no production caller at all), while this one, #793, #794, #795, #797 and #799 are on live paths. That makes this the one I'd rank first — it's the only one in the set that is simultaneously live, security-relevant, and misleadingly marked as having a fix commit.

When fixing: the characterization tests at aes.spec.ts:62 and :68 assert identical ciphertext across different IVs. Both must be inverted, or the fix will fail its own tests and look like a regression.

Audited against `origin/master` — **STILL BROKEN, and the `test(coverage)` commit did not fix it.** Of the eleven small bugs I swept this pass, this is the highest real-world severity. **The commit that references this ticket only pinned it.** `577f78e8` ("test(coverage): wasm-voxel AES-CFB + SECURITY bug #811") touched exactly two files: `aes.spec.ts` (+97) and `COVERAGE-LOOP.md`. **Zero source files.** So anyone searching `git log --grep='#811'` finds a commit that reads like a fix and isn't one. **The defect is unchanged.** `libraries/game/wasm-voxel/src/_components/wasm-voxel/ts/util/aes.ts:137-141` — the round loop still overwrites the state with round keys (`a[i] = this._Ke[r][i]; … t = a.slice();`) rather than applying SubBytes/ShiftRows/MixColumns. The T1–T4 tables are still absent; only the `S` box exists (`:47`). So there is no round function, and the keystream is independent of both input and IV. **And it is live, not latent.** `NetworkManager.ts:276-277` still constructs it for both directions: ```ts this.decryption = new aes.ModeOfOperation.cfb(secretKey, secretKey, 1); this.encryption = new aes.ModeOfOperation.cfb(secretKey, secretKey, 1); ``` Note it also passes `secretKey` as the IV, so key and IV are identical — which compounds the missing round function rather than mitigating it. Whatever this protects has effectively no confidentiality. For context from the same sweep: of the eleven tickets, **#792, #808 and #840 turned out to be latent** (no production caller at all), while this one, #793, #794, #795, #797 and #799 are on live paths. That makes this the one I'd rank first — it's the only one in the set that is simultaneously live, security-relevant, and misleadingly marked as having a fix commit. **When fixing:** the characterization tests at `aes.spec.ts:62` and `:68` assert identical ciphertext across different IVs. Both must be inverted, or the fix will fail its own tests and look like a regression.
Sign in to join this conversation.