Skip to content

md5_hex hex-encodes raw bytes instead of computing MD5 — breaks Last.fm auth and exposes shared_secret reversibly #390

Description

@forkwright

Finding

md5_hex in crates/syndesmos/src/lastfm/auth.rs does not compute MD5. It iterates over the raw bytes of its input and emits each byte as two hex digits, producing a hex encoding of the input rather than a hash of it. Its input is the concatenated signing string "key1value1key2value2...{shared_secret}" built by sign_params. The resulting api_sig is therefore a trivially reversible hex encoding of that concatenation — including the trailing shared_secret — not the MD5 digest the Last.fm signature scheme requires. An in-code comment at line 58 concedes the function is a placeholder ("Full production implementation should use md5 crate"), yet it is wired into live request paths via sign_params and exchange_token.

Evidence

crates/syndesmos/src/lastfm/auth.rs:59 (call site: line 43 in sign_params):

fn md5_hex(data: &[u8]) -> String {
    let mut out = String::with_capacity(data.len() * 2);
    for byte in data {
        write!(out, "{:02x}", byte).ok();
    }
    out
}

crates/syndesmos/src/lastfm/auth.rs:62: same block — the loop hex-encodes each input byte directly with no hash transform. Call site at line 43: md5_hex(input.as_bytes()) passes the full signing string (params + shared_secret) and returns the result as the signature.

Why this matters

Two consequences follow from the broken implementation. First, every api_sig sent over the wire is a reversible hex encoding of the signing string: an observer who captures one request hex-decodes the signature and reads the shared_secret in cleartext, enabling forgery of arbitrary signed requests for the account. This converts a single captured request into full credential compromise. Second, exchange_token always sends an invalid signature, so Last.fm rejects it (error 13) and no session key is ever obtained — the scrobble integration is permanently inoperable at runtime. The existing tests pass only because they assert structural properties (key ordering, param exclusions) that the placeholder satisfies; none assert the digest value against a known-answer vector.

Desired correction

Replace the placeholder with a real MD5 computation using the md5 crate: hash the signing string and format the 16-byte digest as lowercase hex. The sign_params contract (param ordering, exclusions, trailing secret) is already correct. Add a known-answer test using a fixed {api_key, method, token, shared_secret} tuple asserting the exact expected 32-char MD5 hex, so a regression to byte-wise encoding fails the suite. Done when: md5_hex returns the true MD5 hex of its input, a known-answer test pins the signature for a fixed input, and exchange_token obtains a valid session key against a real or mock Last.fm endpoint.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions