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

wasm: fix some string-handling issues in regex/glob matching #2998

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion internal/wasm/sdk/internal/wasm/bindings.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package wasm

// #include <stdlib.h>
//
// extern void opa_println(void *context, int32_t addr);
// extern void opa_abort(void *context, int32_t addr);
// extern int32_t opa_builtin0(void *context, int32_t builtin_id, int32_t ctx);
// extern int32_t opa_builtin1(void *context, int32_t builtin_id, int32_t ctx, int32_t arg0);
Expand All @@ -21,7 +22,12 @@ import (
)

func opaFunctions(imports *wasm.Imports) (*wasm.Imports, error) {
imports, err := imports.AppendFunction("opa_abort", opa_abort, C.opa_abort)
imports, err := imports.AppendFunction("opa_println", opa_println, C.opa_println)
if err != nil {
return nil, err
}

imports, err = imports.AppendFunction("opa_abort", opa_abort, C.opa_abort)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -49,6 +55,11 @@ func opaFunctions(imports *wasm.Imports) (*wasm.Imports, error) {
return imports.AppendFunction("opa_builtin4", opa_builtin4, C.opa_builtin4)
}

//export opa_println
func opa_println(ctx unsafe.Pointer, addr int32) {
getVM(ctx).Println(addr)
}

//export opa_abort
func opa_abort(ctx unsafe.Pointer, addr int32) {
getVM(ctx).Abort(addr)
Expand Down
13 changes: 12 additions & 1 deletion internal/wasm/sdk/internal/wasm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,18 @@ func (i *VM) Abort(arg int32) {
panic("invalid abort argument")
}

panic(abortError{message: string(data[0:n])})
panic(abortError{message: string(data[:n])})
}

// Println is invoked if the policy WASM code calls opa_println().
func (i *VM) Println(arg int32) {
data := i.memory.Data()[arg:]
n := bytes.IndexByte(data, 0)
if n == -1 {
panic("invalid opa_println argument")
}

fmt.Printf("opa_println(): %s\n", string(data[:n]))
}

type builtinError struct {
Expand Down
43 changes: 43 additions & 0 deletions internal/wasm/sdk/opa/opa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,49 @@ func TestOPA(t *testing.T) {
Eval{Result: `set()`},
},
},
// NOTE(sr): The next two test cases were used to replicate issue
// https://github.com/open-policy-agent/opa/issues/2962 -- their raison d'être
// is thus questionable, but it might be good to keep them around a bit.
{
Description: "Only input changing, regex.match",
Policy: `
default hello = false
hello {
regex.match("^world$", input.message)
}`,
Query: "data.p.hello = x",
Evals: []Eval{
Eval{Input: `{"message": "xxxxxxx"}`, Result: `{{"x": false}}`},
Eval{Input: `{"message": "world"}`, Result: `{{"x": true}}`},
},
},
{
Description: "Only input changing, glob.match",
Policy: `
default hello = false
hello {
glob.match("world", [":"], input.message)
}`,
Query: "data.p.hello = x",
Evals: []Eval{
Eval{Input: `{"message": "xxxxxxx"}`, Result: `{{"x": false}}`},
Eval{Input: `{"message": "world"}`, Result: `{{"x": true}}`},
},
},
{
Description: "regex.match with pattern from input",
Query: `x = regex.match(input.re, "foo")`,
Evals: []Eval{
Eval{Input: `{"re": "^foo$"}`, Result: `{{"x": true}}`},
},
},
{
Description: "regex.find_all_string_submatch_n with pattern from input",
Query: `x = regex.find_all_string_submatch_n(input.re, "-axxxbyc-", -1)`,
Evals: []Eval{
Eval{Input: `{"re": "a(x*)b(y|z)c"}`, Result: `{{"x":[["axxxbyc","xxx","y"]]}}`},
},
},
}

for _, test := range tests {
Expand Down
2 changes: 1 addition & 1 deletion wasm/src/glob.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ opa_value *opa_glob_match(opa_value *pattern, opa_value *delimiters, opa_value *
{
re2 = i->second;
} else {
std::string error = glob_translate(opa_cast_string(pattern)->v, opa_cast_string(pattern)->len, v, &re2);
std::string error = glob_translate(p->v, p->len, v, &re2);
if (!error.empty())
{
return NULL;
Expand Down
14 changes: 9 additions & 5 deletions wasm/src/regex.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ opa_value *opa_regex_is_valid(opa_value *pattern)
return opa_boolean(false);
}

std::string pat(opa_cast_string(pattern)->v, opa_cast_string(pattern)->len);
re2::RE2::Options options;
re2::RE2 re(opa_cast_string(pattern)->v, options);
re2::RE2 re(pat, options);
return opa_boolean(re.ok());
}

Expand Down Expand Up @@ -67,15 +68,17 @@ opa_value *opa_regex_match(opa_value *pattern, opa_value *value)
{
return NULL;
}

re2::RE2* re = compile(opa_cast_string(pattern)->v);
std::string pat(opa_cast_string(pattern)->v, opa_cast_string(pattern)->len);
re2::RE2* re = compile(pat.c_str());
if (re == NULL)
{
// TODO: return an error.
return NULL;
}

bool match = re2::RE2::PartialMatch(opa_cast_string(value)->v, *re);
std::string v(opa_cast_string(value)->v, opa_cast_string(value)->len);
bool match = re2::RE2::PartialMatch(v, *re);

reuse(re);
return opa_boolean(match);
}
Expand All @@ -93,7 +96,8 @@ opa_value *opa_regex_find_all_string_submatch(opa_value *pattern, opa_value *val
return NULL;
}

re2::RE2* re = compile(opa_cast_string(pattern)->v);
std::string pat(opa_cast_string(pattern)->v, opa_cast_string(pattern)->len);
re2::RE2* re = compile(pat.c_str());
if (re == NULL)
{
// TODO: return an error.
Expand Down