ovalutil: guard a test element with no object reference - #1976
Open
arpitjain099 wants to merge 1 commit into
Open
ovalutil: guard a test element with no object reference#1976arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
Both DefsToVulns loops read ObjectRef()[0] with no length check, while the StateRef() right below is guarded by len(stateRefs) > 0. The object reference is required by the OVAL schema but the parser does not enforce it, so a feed carrying an rpminfo_test or dpkginfo_test without an <object> child panics the updater with index out of range [0] with length 0 Skip the criterion instead, the same way an object lookup failure is already handled. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Contributor
|
Thanks for the PR, curious; have you seen this in the wild? |
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
2 participants
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.
RPMDefsToVulnsandDpkgDefsToVulnsboth do this:and then, a few lines further down, guard the sibling with
if len(stateRefs) > 0. The comment above explains why the state reference is optional and the object reference is required, but "required by the schema" is not the same as "present in the document": goval-parser does not enforce it, so a test element with no<object>child parses fine and then indexes an empty slice.That happens while unpacking criterions during an update, so a single malformed test element in a fetched OVAL feed takes down the updater rather than costing one vulnerability.
Both call sites now skip the criterion, which is what already happens when the object lookup itself fails. rpm.go logs at debug like the lookup failures around it; dpkg.go increments the existing
stats.Objcounter, which is reported at the end of the run.TestDefsToVulnsWithoutObjectRefparses a minimal OVAL document with the object reference omitted and runs both entry points over it. On the unmodified tree the rpminfo case panics and takes the test binary with it; with this change both return no vulnerabilities and no error.go test ./pkg/ovalutil/passes.