cloudflare / cloudflare/wirefilter

A function's declared parameter type isn't enforced against the value it actually receives (parenthesized field + IsTrue coercion)

Open
#191 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
1.2k
Forks
122
Avg merge
5h 20m
Merged PRs (30d)
4

Description

\`ComparisonExpr::get_type()\` (\`engine/src/ast/field_expr.rs:286-295\`) has this branch:

\`\`\`rust
// engine/src/ast/field_expr.rs:290-292
} else if self.op == ComparisonOpExpr::IsTrue {
// Bool or Array(Bool)
self.lhs.get_type()
\`\`\`

The comment names two expected cases, \`Bool\` or \`Array(Bool)\`. But \`ComparisonExpr::lex_with_lhs\`
(\`field_expr.rs:317-329\`) accepts any LHS whose \`Type::next()\` resolves to \`Bool\` and produces
\`IsTrue\` — which also admits a **\`Map(Bool)\`**-typed field, a third case the comment doesn't cover.
When that happens, \`get_type()\` reports \`Map(Bool)\`, and \`FunctionCallArgExpr::get_type()\`
(\`function_expr.rs:206-214\`) forwards it to \`check_param\` (\`function_expr.rs:460\`), which accepts it
— a \`Map(Bool)\`-declared function parameter type-checks against an expression reporting \`Map(Bool)\`.

What's actually constructed and passed to the function at execute time is
\`LhsValue::Array(result.into())\` (\`function_expr.rs:75\`) — an \`Array\`, not a \`Map\`. The compile-time
type-check and the execute-time value disagree.

**Repro:** register a function \`f\` with a \`Map(Bool)\` parameter. \`f(m) == "x"\` (no parens) correctly
delivers a \`Map\`. \`f((m)) == "x"\` (parenthesized) type-checks identically but delivers an \`Array\`
instead — any function that trusts the declared parameter type instead of matching on the actual
\`LhsValue\` variant it receives will observe the wrong one.

To be clear about scope: this isn't a memory-safety issue — \`LhsValue\` is a plain safe enum and
nothing in the function-call path uses \`unsafe\` casts on it. It's a type-contract gap: the checked
type and the delivered value can disagree, silently.

**Suggested fix:** either restrict the \`IsTrue\` coercion to genuinely scalar \`Bool\` (excluding
\`Map(Bool)\`/\`Array(Bool)\`) — matching what the \`// Bool or Array(Bool)\` comment already assumes — or
verify the runtime \`LhsValue\` variant matches the declared parameter type before invoking the
function's closure.

Possibly related to #167.

Found with the [rust-in-peace](https://github.com/scadastrangelove/rust-in-peace) pipeline.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.