spikerj/spikersoft-issues
1
0 Fork 0
Code Issues 23 Pull Requests Actions Packages Projects Releases Wiki Activity

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

New Issue
Closed
opened 2026-07-23 05:47:02 +00:00 by spikerj · 2 comments
spikerj commented 2026-07-23 05:47:02 +00:00
Owner
Copy link
Copy source

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).
spikerj commented 2026-07-29 18:20:32 +00:00
Author
Owner
Copy link
Copy source

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:

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.
spikerj commented 2026-08-07 13:47:26 +00:00
Author
Owner
Copy link
Copy source

Migrated to spikerj/spikersoft-angular#637 as part of the umbrella-tracker breakup.

Verified 2026-08-07 against the synced default branch.

  • Code: spikersoft-angular@8e5a4048 — still broken, and the commit that references this ticket did not fix it: 577f78e8 ("test(coverage): wasm-voxel AES-CFB + SECURITY bug #811") touched only aes.spec.ts and COVERAGE-LOOP.md — zero source files. libraries/game/wasm-voxel/src/_components/wasm-voxel/ts/util/aes.ts:137-141 still overwrites the state with round keys (a[i] = this._Ke[r][i]; … t = a.slice();) instead of applying the AES round function; rg 'T1' over the file returns nothing, so the T1–T4 tables are still absent. Live path confirmed: ts/net/minecraft/client/network/NetworkManager.ts:276-277 still constructs new aes.ModeOfOperation.cfb(secretKey, secretKey, 1) for both directions — key and IV identical.
  • Live: Not externally probed (client-side crypto), but the construction site is on the live connection path, so the keystream is fixed per key and reused for every block, and the IV has no effect.
  • Remaining: Restore a correct AES round function (port real aes-js with its T1–T4 tables, or swap to WebCrypto / a vetted aes-js dependency). Then invert the characterization assertions at aes.spec.ts:62 and :68 (they currently assert identical ciphertext across different IVs) or the fix will fail its own tests and look like a regression. Highest real-world severity of the coverage-wave bug set: live, security-relevant, and misleadingly marked as having a fix commit.

Status: still broken — full detail carried over to the new issue.

Closing here. Work now lives in the repo that holds the fix, so fixes #637 in a PR will
auto-close it on merge. The umbrella tracker keeps cross-repo epics only.

— Opus 5 Agent

Migrated to **spikerj/spikersoft-angular#637** as part of the umbrella-tracker breakup. Verified 2026-08-07 against the synced default branch. - **Code:** `spikersoft-angular@8e5a4048` — still broken, and **the commit that references this ticket did not fix it**: `577f78e8` ("test(coverage): wasm-voxel AES-CFB + SECURITY bug #811") touched only `aes.spec.ts` and `COVERAGE-LOOP.md` — zero source files. `libraries/game/wasm-voxel/src/_components/wasm-voxel/ts/util/aes.ts:137-141` still overwrites the state with round keys (`a[i] = this._Ke[r][i]; … t = a.slice();`) instead of applying the AES round function; `rg 'T1'` over the file returns nothing, so the T1–T4 tables are still absent. Live path confirmed: `ts/net/minecraft/client/network/NetworkManager.ts:276-277` still constructs `new aes.ModeOfOperation.cfb(secretKey, secretKey, 1)` for both directions — key and IV identical. - **Live:** Not externally probed (client-side crypto), but the construction site is on the live connection path, so the keystream is fixed per key and reused for every block, and the IV has no effect. - **Remaining:** Restore a correct AES round function (port real aes-js with its T1–T4 tables, or swap to WebCrypto / a vetted `aes-js` dependency). Then **invert** the characterization assertions at `aes.spec.ts:62` and `:68` (they currently assert identical ciphertext across different IVs) or the fix will fail its own tests and look like a regression. Highest real-world severity of the coverage-wave bug set: live, security-relevant, and misleadingly marked as having a fix commit. Status: still broken — full detail carried over to the new issue. Closing here. Work now lives in the repo that holds the fix, so `fixes #637` in a PR will auto-close it on merge. The umbrella tracker keeps cross-repo epics only. — Opus 5 Agent
Sign in to join this conversation.
No Branch/Tag Specified
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 epic
Cross-repo epic — children live in per-repo trackers
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#811