March 10, 2021·4 min read·engineeringgosecurityparsing

Go 1.16.1 fixes progress failures in ZIP and XML readers

Two standard library fixes address a panic during ZIP filename normalization and an infinite loop when a custom XML token reader ends inside an element.

A loop can appear to consume its input while checking a condition that never changes. The ZIP security fix in Go 1.16.1 is a particularly small example. Its filename-normalization loop removes parent-directory prefixes from one variable and checks a different variable for the next iteration. The patch changes the variable in the condition, restoring the relationship between the loop's test and its progress.

The release history entry for Go 1.16.1 records its March 10 release and security fixes in archive/zip and encoding/xml. The same-day security announcement gives the affected APIs and identifies CVE-2021-27919 for ZIP and CVE-2021-27918 for XML. These are availability failures with specific preconditions. Understanding those preconditions matters more than treating every use of either package as equivalent.

I value explicit operational behavior in Go, including errors that callers can handle. Both fixes matter because input that should produce a bounded result can instead disrupt the operation. The underlying language does not remove the need to reason about parser state and termination.

The ZIP failure occurs while adapting names

The announcement identifies Reader.Open as the affected ZIP API and notes that it is new in Go 1.16. Opening an archive containing a filename beginning with ../ can cause a panic. This description is narrower than a claim that every read of a ZIP archive fails or that the defect writes outside an extraction directory.

The ZIP fix shows the loop inside toValidName. Its working variable is p. The old condition examines name, while the loop body shortens p. Once name starts with the relevant prefix, shortening p does not make that condition false.

The corrected fragment checks the variable that the body changes. This excerpt reproduces the repaired loop from the patch.

for strings.HasPrefix(p, "../") {
    p = p[len("../"):]
}

The difference is one identifier, but the invariant changes. Each successful test establishes that the current working string has a removable prefix. Removing it then moves that same string toward a state where the test is false. In the old version, the condition establishes a fact about an unchanged input instead.

The regression test makes the behavior concrete. It constructs an archive whose only entry is named ../test.txt, creates a reader, and opens test.txt. That is evidence about the filename mapping the API performs. It also puts the problematic filename inside an actual archive, so the test covers the surrounding reader behavior.

A caller assessing exposure needs to locate calls to Reader.Open and determine who controls the archive. A program that accepts externally supplied ZIP files can encounter a filename it did not choose. Trusted embedded data has a different input boundary, although the library still needs to handle its documented cases correctly. The advisory does not describe this defect as arbitrary file extraction or code execution.

XML decoding needs to distinguish tokens from exhaustion

The XML advisory concerns decoders created with xml.NewTokenDecoder and a custom xml.TokenReader. Decode, DecodeElement, and Skip can enter an infinite loop if that reader returns EOF while an XML element remains open. The condition includes both the custom reader and the structural state of the XML stream.

The XML patch changes how Decoder.Token handles the token and error returned by its underlying source. When no token remains and an error arrives, EOF inside an open element becomes a syntax error. When there is still a token to process, the decoder clears the accompanying error and processes that token.

This is a distinction between the end of a source and completion of the structure being read. A source can stop without supplying the closing token an element requires. The decoder must preserve the structural failure instead of treating the source's exhaustion as a reason to continue waiting for meaningful tokens.

The tests examine more than one arrangement of EOF. Valid token sequences include an opening element followed by its matching end. A malformed sequence adds a different opening element before the first element ends. The test exercises early and late EOF behavior and expects a syntax error for the malformed sequence.

Another added test uses a token reader that returns nil with no error before returning EOF. The patch's comment calls this legal but discouraged. Testing it in strict and non-strict modes checks a detail that a straightforward byte-stream example would miss. A reusable token consumer has to account for the reader interface's permitted results, including inconvenient ones.

I find these tests useful as an explanation of the change. They describe the boundary between a reader's result and the decoder's own state. An implementation can obey the return shape of an interface while still mishandling what those returns mean together. A test that supplies only complete documents cannot establish this behavior. The added cases make the missing end token and the timing of EOF explicit inputs to the decoder.

Updating the compiler is part of updating the program

The security announcement releases Go 1.15.9 alongside Go 1.16.1 and recommends that all users update to one of them. It recommends Go 1.16.1 when the choice is unclear. The older branch matters because the XML failure is not confined to the new ZIP filesystem API introduced in Go 1.16.

For an application, the useful inventory therefore includes the build toolchain and the affected call paths. Installing a fixed toolchain changes subsequent builds. A deployed executable needs the repaired library code incorporated into its build before that executable benefits from the fix. This follows from the fixes being changes to standard library implementation code.

The two patches also suggest different review questions. ZIP normalization needs a termination condition tied to the value being shortened. XML decoding needs a terminal error tied to an incomplete structural state. Both questions are specific enough to examine in source and regression tests. Their answers explain why these small maintenance changes alter whether an operation finishes or fails in a controlled way.