apk: guard against short lines in installed database#1949
Open
arpitjain099 wants to merge 1 commit into
Open
Conversation
The record parser slices every line as line[2:] to get the value, which assumes each line is at least a one byte key plus a ":". A line that's just a newline is 1 byte, so it panics with "slice bounds out of range [2:1]" and takes down the scan. An installed database with a stray blank line inside a record produces exactly that: the "\n\n" split leaves the extra newline at the head of the next record. Layer contents are attacker controlled, so this is reachable from any indexed image. Skip lines too short to hold a key and a value. The dpkg scanner gets this for free from net/textproto, which apk can't use because its keys are case sensitive. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I do supply-chain security research, mostly on scanners and indexers.
The apk record loop in
apk/scanner.goslices every line asline[2:]. A bare newline is a 1 byte line, so that'sslice bounds out of range [2:1]and the scan panics.You get one from a stray blank line inside a record. Splitting on
"\n\n"leaves the extra newline sitting at the front of the next record, and its firstReadByteshands back just"\n". Layer contents come from whatever image is being indexed, so a malformed or craftedlib/apk/db/installedis enough to crash the indexer.Fix is a length check before the slice. dpkg doesn't have this problem because textproto tolerates the junk for it, which the comment above the loop already explains apk can't use.
Test is
TestBlankLine, a tarred-up installed database with an extra blank line, run throughScanthe normal way. Panics on main, passes with the fix, and no network needed.go test ./apk/andgo test -race ./apk/are clean.