Files

T

kyle 7755be49e9 Fix SHA-256 padding bug for messages of length 63 mod 64

pad_message wrote 0x00 into the last byte of the current block and
deferred the 0x80 pad byte to the next block when mbi == 63, producing
incorrect digests for every message whose length is 63 mod 64 (and
correspondingly wrong HMAC tags). Always write the pad byte in the
current block first, then flush an extra block only when the 8-byte
length field no longer fits.

Adds regression tests for SHA-256 boundary lengths 55/56/63/64/65 and
HMAC with a 63-byte message. Bumps to 1.0.4 for crates.io publication.

Ref: docs/LAGUNA-AUDIT.md

2026-09-17 12:25:23 -07:00

2.9 KiB

Raw Permalink Blame History

Laguna audit: local SHA-256 padding fix (1.0.4)

Date: 2026-09-17 Mode: local uncommitted changes vs HEAD Scope: Cargo.toml, Cargo.lock, src/sha256.rs, tests/sha256.rs, tests/hmac.rs Diff: 5 files, +74 / −21 Verdict: Correct. No bugs. Two minor suggestions addressed.

Summary

This change fixes a SHA-256 padding bug for messages whose length is 63 mod 64, bumps the crate from 1.0.3 to 1.0.4, adds regression tests, and removes the publish = ["kellnr"] restriction for crates.io publication.

The old pad_message in src/sha256.rs used the condition self.mbi < MB_SIZE - 1 to decide whether to write the 0x80 pad byte in the current block before zero-fill and block processing. When mbi == 63 (i.e. the message is 63 mod 64 bytes), that condition was false, so position 63 was zero-filled instead of receiving 0x80. The block was then processed with message || 0x00, and 0x80 was written at index 0 of the next block — producing a digest of message || 0x00 || 0x80 || ... instead of the correct message || 0x80 || 0x00 || ....

The new code always writes the pad byte at self.mb[self.mbi] first (safe because update flushes full blocks, so mbi is always < MB_SIZE on entry), then zero-fills and processes the current block only when the 8-byte length field no longer fits (mbi > MB_SIZE - 8).

HMAC-SHA-256 was affected too, since its inner hash processes 64-byte ipad + message bytes; a 63-byte message makes the inner length 127 (63 mod 64).

Verification

The six new test vectors match sha256sum and Python hmac/hashlib exactly: SHA-256 at lengths 55, 56, 63, 64, 65 bytes (all a), and HMAC-SHA-256 over a 63-byte message with the 20-byte 0x0b key.
cargo test passes: 11 tests across unit, integration, and doc suites.
cargo clippy --all-targets is clean.

Issues

1 — resolved — comment refinement: `src/sha256.rs:177`

The branch comment originally restated the code ("Zero-fill and process it, then start a fresh block for the length"). Trimmed to state the why:

// Fewer than 8 bytes remain; flush this block and start
// a fresh block for the length field.

2 — resolved — comment refinement: `tests/hmac.rs:63`

The test comment referenced "the length that triggered the padding bug" (bug history) rather than the invariant. Reworded to state the invariant:

// A 63-byte message makes the inner SHA-256 process 127 bytes
// (64 key-pad + 63 message), which is 63 mod 64 — the boundary
// where the pad byte must start a new block.

Packaging change

Removing publish = ["kellnr"] from Cargo.toml is intentional so cargo publish targets crates.io for the 1.0.4 release.

Counts

Severity	Count	State
bug	0	—
suggestion	2	resolved
packaging	1	intentional

2.9 KiB Raw Permalink Blame History Unescape Escape