Skip to content

CSSCodec / EncodingPatternPreservation: predictable placeholder marker can desync restoreOriginalContent() #915

Description

@Socialpranker

Summary

EncodingPatternPreservation (used by CSSCodec.encode() to shield rgb(...) triplets from CSS-escaping) uses a fixed, publicly-known default marker — this class's own simple name, the literal string "EncodingPatternPreservation" — as a temporary placeholder for matched content during encoding.

restoreOriginalContent() restores the captured content afterwards via replaceFirst(marker, ...), matching the marker as plain literal text, in FIFO order. Since the marker is a fixed constant, if the input already contains that literal string ahead of real matched content, replaceFirst matches the attacker-supplied text instead of the real placeholder — every subsequent restoration in that call shifts by one, and the output is silently corrupted.

Repro

Pattern rgb = Pattern.compile("([rR][gG][bB])\\s*\\(\\s*\\d{1,3}\\s*,\\s*\\d{1,3}\\s*,\\s*\\d{1,3}\\s*\\)");
EncodingPatternPreservation epp = new EncodingPatternPreservation(rgb);

String input = "EncodingPatternPreservation color:rgb(1,2,3);";
String replaced = epp.captureAndReplaceMatches(input);
// replaced == "EncodingPatternPreservation color:EncodingPatternPreservation;"

String restored = epp.restoreOriginalContent(replaced);
// restored == "rgb(1,2,3) color:EncodingPatternPreservation;"   (desynced)
// expected == "EncodingPatternPreservation color:rgb(1,2,3);"

Same effect reachable end-to-end through CSSCodec.encode() with equivalent input.

Severity

I want to be precise here rather than overclaim: the only regex currently routed through this class is CSSCodec's RGB-triplet pattern, which can only ever capture digits, %, commas, whitespace, and rgb(/). A desynchronized restore therefore can't smuggle unexpected/dangerous characters through encodeForCSS() — I don't believe this is an XSS or encoding-bypass primitive. It's an output-integrity bug: encodeForCSS() can silently return corrupted/scrambled output for adversarial input, without raising any error, which could still surprise a caller relying on positional/structural correctness of the encoded result.

Fix

I've opened a PR with a minimal fix: derive the default marker per-instance from a random UUID instead of the fixed literal, so it can't be predicted or pre-supplied in input (hyphens stripped, since CSSCodec.encode() itself backslash-escapes hyphens before restoration runs — confirmed via the existing CSSCodecTest suite). Public setReplacementMarker() API unchanged. Includes 2 new regression tests plus updates to the 2 existing tests that asserted on the old fixed-marker string.

Fixes/relates to this PR: #914

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions