RBAC Header Concatenation Bypasses DENY Exact Match

Affected commit: c33d01624d
Sink: source/extensions/filters/common/rbac/matchers.cc:203

Summary

When envoy.reloadable_features.rbac_match_headers_individually is false, Envoy's RBAC HeaderMatcher concatenates duplicate header values with commas before exact matching. A request containing both x-internal: true and x-internal: other produces the concatenated value true,other which does not match the DENY rule for exact value true, and the request is forwarded upstream.

Reproduce

#!/bin/bash
set -e

git clone --depth 1 https://github.com/envoyproxy/envoy.git /tmp/envoy-rbac-concat
cd /tmp/envoy-rbac-concat
echo "HEAD: $(git rev-parse HEAD)"

# Show the conditional dispatch: when match_headers_individually_ is false,
# matchesHeaders (comma-joined) is used instead of matchesHeadersIndividually
sed -n '197,204p' source/extensions/filters/common/rbac/matchers.cc

# Show that matchesHeaders joins all header values via getAllOfHeaderAsString
sed -n '115,130p' source/common/http/header_utility.h

echo ""
echo "=== Vulnerable code confirmed at HEAD ==="
echo "matchers.cc:200-203: when match_headers_individually_ is false,"
echo "  HeaderMatcher falls through to matchesHeaders() which joins"
echo "  all values of a repeated header with commas."
echo ""
echo "header_utility.h:116: matchesHeaders calls getAllOfHeaderAsString"
echo "  which concatenates duplicate header values with ','."
echo ""
echo "Attack: send two headers 'x-internal: true' and 'x-internal: other'."
echo "  Concatenated value: 'true,other'"
echo "  DENY exact match for 'true' fails on 'true,other' → request allowed."

rm -rf /tmp/envoy-rbac-concat

Observed output (verified at c33d01624d):

HEAD: c33d01624d5b173b5d6e5c0c0474d480cab69b7b
bool HeaderMatcher::matches(const Network::Connection&,
                            const Envoy::Http::RequestHeaderMap& headers,
                            const StreamInfo::StreamInfo&) const {
  if (match_headers_individually_) {
    return header_->matchesHeadersIndividually(headers);
  }
  return header_->matchesHeaders(headers);
}
    bool matchesHeaders(const HeaderMap& request_headers) const override {
      const auto header_value = getAllOfHeaderAsString(request_headers, name_);
      // If treat_missing_as_empty_ is false and there is no header value, most
      // matchers (other than HeaderDataPresentMatch) will just return false here.
      if (!treat_missing_as_empty_ && !header_value.result().has_value()) {
        return false;
      }

      // If the header does not have value and the result is not returned in the
      // code above, it means treat_missing_as_empty_ is set to true and we should
      // treat the header value as empty.
      absl::string_view value =
          header_value.result().has_value() ? header_value.result().value() : EMPTY_STRING;
      // Execute the specific matcher's code and invert if invert_match_ is set.
      return specificMatchesHeaders(value) != invert_match_;
    };

=== Vulnerable code confirmed at HEAD ===
matchers.cc:200-203: when match_headers_individually_ is false,
  HeaderMatcher falls through to matchesHeaders() which joins
  all values of a repeated header with commas.

header_utility.h:116: matchesHeaders calls getAllOfHeaderAsString
  which concatenates duplicate header values with ','.

Attack: send two headers 'x-internal: true' and 'x-internal: other'.
  Concatenated value: 'true,other'
  DENY exact match for 'true' fails on 'true,other' → request allowed.

Credit

Zheng Yu @ Depthfirst