Return failure on comment mismatch in CLI decompression

[0xazanul]

Summary

• Fix missing return BROTLI_FALSE in the final comment verification check of DecompressFile() (c/tools/brotli.c:1290-1297)
• The -C flag now correctly rejects streams with missing or mismatched comments

The Bug

In DecompressFile(), the final comment state check at the end of successful decoding detects a mismatch but does not act on it:

/* Final check */
if (context->comment_state != COMMENT_OK) {
    fprintf(stderr, "corrupt input [%s]\n", ...);
    if (context->verbosity > 0) {
        fprintf(stderr, "reason: comment mismatch\n");
    }
}
return BROTLI_TRUE;  // ← unconditional, should be inside an else branch

The error message is printed to stderr, but the function returns BROTLI_TRUE regardless. The caller DecompressFiles() then keeps the output file and the process exits 0.

When this triggers

The early check (line 1231) only catches COMMENT_BAD mid-stream. The final check catches states that the early check misses:

Scenario	State at final check	Early check	Final check
Stream has wrong comment bytes	COMMENT_BAD	✅ caught	n/a
Stream has no metadata block	COMMENT_INIT	missed	prints error, returns TRUE
Stream has incomplete metadata	COMMENT_READ	missed	prints error, returns TRUE

A tampered or re-encoded stream that strips the metadata block bypasses verification entirely.

Impact

Scripts relying on -C to validate stream integrity/fingerprint accept tampered streams:

# Intended: reject if comment doesn't match
brotli -d -C "$EXPECTED_FINGERPRINT" input.br -o output && process output
# Actual: always succeeds, "process output" always runs

The Fix

One-line change — add return BROTLI_FALSE inside the mismatch branch:

if (context->comment_state != COMMENT_OK) {
    fprintf(stderr, "corrupt input [%s]\n", ...);
    if (context->verbosity > 0) {
        fprintf(stderr, "reason: comment mismatch\n");
    }
    return BROTLI_FALSE;  // ← added
}
return BROTLI_TRUE;

This is consistent with every other error path in the function (lines 1237, 1246, 1280, 1307).

Test plan

• Encode a file with -C <comment>, decode with matching -C → succeeds (no change)
• Encode a file with -C <comment>, decode with wrong -C → now fails with exit code 1
• Encode a file without -C, decode with -C <comment> → now fails (no metadata in stream, COMMENT_INIT at final check)
• Decode without -C → succeeds (no change, comment_state initialized to COMMENT_OK)

[eustas]

Awesome, thanks!

[0xazanul]

Hey if i report this in vrp google will this qualify?

[eustas]

Likely not. This feature was requested by community,... but it seems to be not actually used (otherwise problem should have been already reported).