SDK PR #87 — L2PS nonce reuse fix (3 unresolved security findings)

[linear]

PR: kynesyslabs/sdks#87
Branch: fix/l2ps-nonce-reuse → main

What it does

Switches L2PS encryptTx from a static IV reused across every encrypt call to a fresh 12-byte AES-GCM nonce per call. Adds optional nonce?: string (base64) to L2PSEncryptedPayload for forward compatibility; decryptTx falls back to the legacy this.iv when the field is absent.

Why urgent

Before this PR, every L2PS subnet reused the same IV across every encrypted tx. AES-GCM with nonce reuse is a complete confidentiality + authentication break — once two ciphertexts share an IV, an attacker can XOR them to recover plaintext deltas, and the GHASH authentication key leaks deterministically. The bug was latent until PR #5 made L2PS payloads leave the demo process (broadcast to chain); landing this PR is required before any L2PS subnet sees production traffic.

CI status

All green:

• Greptile Review: success
• SonarCloud: success
• CodeQL × 3: success
• CodeRabbit: 1 actionable comment

Unresolved AI findings — must address before merge

🔒 Greptile P2 security (l2ps.ts:216, encrypt)

The AES-GCM nonce embedded in payload.nonce is not authenticated — stored as a plain field, not passed as additionalData. AES-GCM auth covers ciphertext + AAD but not the IV. An adversary modifying stored payload can flip the nonce field, forcing decryption with wrong IV; auth tag always fails → targeted DoS against specific ciphertexts. Pass raw nonce bytes as additionalData on both encrypt and decrypt to bind it to the authenticated envelope.

Suggested fix:

const nonce = forge.random.getBytesSync(12);
cipher.start({ iv: nonce, additionalData: nonce });

🔒 Greptile P2 security (l2ps.ts:312, decrypt)

Mirror the additionalData on decipher.start when payload carries a per-call nonce — otherwise any ciphertext produced after the encrypt-side AAD fix would always fail authentication.

Suggested fix:

decipher.start({
    iv,
    tag,
    ...(encryptedPayload.nonce ? { additionalData: iv } : {})
});

🟡 CodeRabbit (l2ps.ts:306, decrypt)

encryptedPayload.nonce falsy fallback misclassifies nonce: "" as legacy. Also accepts any decoded length even though the contract is 12 bytes. Use strict-undefined check + validate 12-byte length; fail fast on malformed nonces.

Suggested fix:

let iv: forge.Bytes;
if (encryptedPayload.nonce === undefined) {
    iv = this.iv;
} else {
    iv = forge.util.decode64(encryptedPayload.nonce);
    if (iv.length !== 12) {
        throw new Error('Invalid encrypted payload nonce: expected a base64-encoded 12-byte value');
    }
}

Action items

• Apply AAD binding on encrypt (Greptile Fees development #1)
• Apply AAD binding on decrypt (Greptile Splitting transactions from blocks #2)
• Strict-undefined check + 12-byte length validation on decrypt nonce (CodeRabbit)
• Add a round-trip test that verifies AAD binding (tampering nonce → auth fail)
• Re-run existing L2PS smoke tests after AAD change to ensure backward compat with legacy payloads

Blocks

Merge of this PR blocks any production L2PS subnet rollout. Until then, all L2PS demo traffic stays on the local-broadcast simulation path that never sends ciphertexts off-process.