New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
topdown/json: Fix panic in json.filter on empty JSON paths. #5200
topdown/json: Fix panic in json.filter on empty JSON paths. #5200
Conversation
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
19fee81
to
388b9c4
Compare
This commit fixes a panic discovered in the `json.filter` builtin that could be triggered with an empty JSON path parameter, such as `""`. This panic was caused by indexing logic in a helper function always assuming it had at least one path segment to work with, and thus indexing out-of-bounds when no path segment was present. The issue was fixed by adding an extra check to the helper function for the null path case, and adding new unit tests to check for the issue. Fixes: open-policy-agent#5199 Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
388b9c4
to
d1acc16
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good find!
topdown/json.go
Outdated
@@ -203,6 +203,11 @@ func pathsToObject(paths []ast.Ref) ast.Object { | |||
node := root | |||
var done bool | |||
|
|||
// If it's a null JSON path, skip all further processing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, but it's an empty string, not null.. right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I wrote that, I was thinking about how the empty string in the list of paths-to-be-filtered is translated into a blank ast.Ref
, which is basically a null/nil value. I'll embellish the comment with more details.
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
…icy-agent#5200) This commit fixes a panic discovered in the `json.filter` builtin that could be triggered with an empty JSON path parameter, such as `""`. This panic was caused by indexing logic in a helper function always assuming it had at least one path segment to work with, and thus indexing out-of-bounds when no path segment was present. The issue was fixed by adding an extra check to the helper function for the null path case, and adding new unit tests to check for the issue. Fixes: open-policy-agent#5199 Signed-off-by: Philip Conrad <philipaconrad@gmail.com> Signed-off-by: Byron Lagrone <byron.lagrone@seqster.com>
This commit fixes a panic discovered in the
json.filter
builtin that could be triggered with an empty JSON path parameter, such as""
. This panic was caused by indexing logic in a helper function always assuming it had at least one path segment to work with, and thus indexing out-of-bounds when no path segment was present.The issue was fixed by adding an extra check to the helper function for the null path case, and adding new unit tests to check for the issue.
Fixes: #5199