Skip to content
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

Fix issues discovered through fuzz testing #2659

Merged
merged 2 commits into from Aug 25, 2020

Conversation

tsandall
Copy link
Member

@tsandall tsandall commented Aug 25, 2020

This PR contains a few combined commits that fix panics found through fuzz testing.

The type checker was panicing when given a reference to an object with
a composite key because internally it infers the key type using
ast.ValueToInterface (which returns map[string]interface{} and not
map[interface{}]interface{}). This change updates the types package to
support map[string]interface{} internally.

Fixing the TypeOf function resolved the panic but it surfaced another
issue in the type checker where errors were returned if objects or
arrays were dereferenced with composite keys--in case of arrays, the
result is undefined but in the case of objects, this is valid.

Fixes open-policy-agent#2648

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Previously the compiler was not rewriting expr terms in the rule
args. In particular, this meant that indirect refs were not being
rewritten. This would lead to panics in the safety check which assume
that indirect refs have been rewritten (in other words, that reference
heads are always variables.)

This commit just updates the expr term rewriting to process rule args
as it does for the rule head key and value terms.

Fixes open-policy-agent#2649

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
@tsandall tsandall changed the title types: Fix panic on reference to object with composite key Fix issues discovered through fuzz testing Aug 25, 2020
@tsandall tsandall merged commit 31224c4 into open-policy-agent:master Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants