Header Matching Comma-Join Defeats Route RBAC

Affected commit: c33d01624d
Sink: source/common/http/header_utility.h:116

Summary

When individual header matching is disabled, Envoy joins repeated header values with commas before exact matching. An attacker can duplicate a routing header to prevent a protected route from matching, causing fall-through to an unprotected route.

Reproduce

#!/bin/bash
set -e

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

# Show matchesHeaders uses getAllOfHeaderAsString (comma-joined)
sed -n '115,130p' source/common/http/header_utility.h

# Show getAllOfHeaderAsString joins with comma
grep -n 'getAllOfHeaderAsString' source/common/http/header_utility.cc | head -5
grep -n -A3 'absl::StrJoin\|result_.emplace' source/common/http/header_utility.cc | head -15

echo ""
echo "=== Vulnerable code confirmed at HEAD ==="
echo "header_utility.h:116: matchesHeaders() calls"
echo "  getAllOfHeaderAsString() which joins repeated header values"
echo "  with commas before exact matching."
echo ""
echo "When a route uses header-based matching with exact values and"
echo "individual matching is disabled, an attacker can duplicate the"
echo "routing header to produce a comma-joined value that fails the"
echo "exact match, causing fall-through to an unprotected route."

rm -rf /tmp/envoy-header-join

Observed output (verified at c33d01624d):

HEAD: c33d01624d5b173b5d6e5c0c0474d480cab69b7b
    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 ===
header_utility.h:116: matchesHeaders() calls
  getAllOfHeaderAsString() which joins repeated header values
  with commas before exact matching.

When a route uses header-based matching with exact values and
individual matching is disabled, an attacker can duplicate the
routing header to produce a comma-joined value that fails the
exact match, causing fall-through to an unprotected route.

Credit

Zheng Yu @ Depthfirst