This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 03391d2be703aa621d690034f06090e6c3b3f579 Author: Masakazu Kitajo <[email protected]> AuthorDate: Mon Sep 30 16:27:15 2024 -0600 header_rewrite: Fix condition parser (#11785) * header_rewrite: Fix condition parser * Print error if operator type is unknown * Fix buffer overflow * Add tests for IP range (cherry picked from commit b0353197c3b89e637b532488c63fceb1ff3db018) --- plugins/header_rewrite/condition.cc | 18 ++++++++++++++---- plugins/header_rewrite/header_rewrite_test.cc | 22 ++++++++++++++++++++++ plugins/header_rewrite/matcher.h | 1 + plugins/header_rewrite/parser.cc | 11 ++++++++++- plugins/header_rewrite/ruleset.cc | 5 +++++ 5 files changed, 52 insertions(+), 5 deletions(-) diff --git a/plugins/header_rewrite/condition.cc b/plugins/header_rewrite/condition.cc index b3e87a4d92..b5f84eedfe 100644 --- a/plugins/header_rewrite/condition.cc +++ b/plugins/header_rewrite/condition.cc @@ -43,13 +43,23 @@ parse_matcher_op(std::string &arg) break; case '/': arg.erase(0, 1); - arg.erase(arg.length() - 1, arg.length()); - return MATCH_REGULAR_EXPRESSION; + // There should be a slash at the end + if (arg.length() >= 1 && arg[arg.length() - 1] == '/') { + arg.erase(arg.length() - 1, arg.length()); + return MATCH_REGULAR_EXPRESSION; + } else { + return MATCH_ERROR; + } break; case '{': arg.erase(0, 1); - arg.erase(arg.length() - 1, arg.length()); - return MATCH_IP_RANGES; + // There should be a right brace at the end + if (arg.length() >= 1 && arg[arg.length() - 1] == '}') { + arg.erase(arg.length() - 1, arg.length()); + return MATCH_IP_RANGES; + } else { + return MATCH_ERROR; + } default: return MATCH_EQUAL; break; diff --git a/plugins/header_rewrite/header_rewrite_test.cc b/plugins/header_rewrite/header_rewrite_test.cc index 4709db1229..355ec164b3 100644 --- a/plugins/header_rewrite/header_rewrite_test.cc +++ b/plugins/header_rewrite/header_rewrite_test.cc @@ -235,6 +235,28 @@ test_parsing() END_TEST(); } + { + ParserTest p(R"(cond %{INBOUND:REMOTE-ADDR} {192.168.201.0/24,10.0.0.0/8})"); + + CHECK_EQ(p.getTokens().size(), 3UL); + CHECK_EQ(p.getTokens()[0], "cond"); + CHECK_EQ(p.getTokens()[1], "%{INBOUND:REMOTE-ADDR}"); + CHECK_EQ(p.getTokens()[2], "{192.168.201.0/24,10.0.0.0/8}"); + + END_TEST(); + } + + { + ParserTest p(R"(cond %{INBOUND:REMOTE-ADDR} { 192.168.201.0/24,10.0.0.0/8 })"); + + CHECK_EQ(p.getTokens().size(), 3UL); + CHECK_EQ(p.getTokens()[0], "cond"); + CHECK_EQ(p.getTokens()[1], "%{INBOUND:REMOTE-ADDR}"); + CHECK_EQ(p.getTokens()[2], "{ 192.168.201.0/24,10.0.0.0/8 }"); + + END_TEST(); + } + { ParserTest p("add-header X-HeaderRewriteApplied true"); diff --git a/plugins/header_rewrite/matcher.h b/plugins/header_rewrite/matcher.h index 877cb05dae..aec4c98575 100644 --- a/plugins/header_rewrite/matcher.h +++ b/plugins/header_rewrite/matcher.h @@ -39,6 +39,7 @@ enum MatcherOps { MATCH_GREATER_THEN, MATCH_REGULAR_EXPRESSION, MATCH_IP_RANGES, + MATCH_ERROR, }; // Condition modifiers diff --git a/plugins/header_rewrite/parser.cc b/plugins/header_rewrite/parser.cc index a7bc85947c..5be73a22ba 100644 --- a/plugins/header_rewrite/parser.cc +++ b/plugins/header_rewrite/parser.cc @@ -28,7 +28,7 @@ #include "parser.h" -enum ParserState { PARSER_DEFAULT, PARSER_IN_QUOTE, PARSER_IN_REGEX, PARSER_IN_EXPANSION }; +enum ParserState { PARSER_DEFAULT, PARSER_IN_QUOTE, PARSER_IN_REGEX, PARSER_IN_EXPANSION, PARSER_IN_BRACE }; bool Parser::parse_line(const std::string &original_line) @@ -88,6 +88,15 @@ Parser::parse_line(const std::string &original_line) _empty = true; return false; } + } else if ((state == PARSER_DEFAULT) && ((i == 0 || line[i - 1] != '%') && line[i] == '{')) { + state = PARSER_IN_BRACE; + extracting_token = true; + cur_token_start = i; + } else if ((state == PARSER_IN_BRACE) && (line[i] == '}')) { + cur_token_length = i - cur_token_start + 1; + _tokens.push_back(line.substr(cur_token_start, cur_token_length)); + state = PARSER_DEFAULT; + extracting_token = false; } else if (!extracting_token) { if (_tokens.empty() && line[i] == '#') { // this is a comment line (it may have had leading whitespace before the #) diff --git a/plugins/header_rewrite/ruleset.cc b/plugins/header_rewrite/ruleset.cc index c25703022b..dca1139bbe 100644 --- a/plugins/header_rewrite/ruleset.cc +++ b/plugins/header_rewrite/ruleset.cc @@ -54,6 +54,11 @@ RuleSet::add_condition(Parser &p, const char *filename, int lineno) TSHttpHookNameLookup(_hook), p.get_op().c_str(), p.get_arg().c_str()); return false; } + if (c->get_cond_op() == MATCH_ERROR) { + delete c; + TSError("[%s] in %s:%d: Invalid operator", PLUGIN_NAME, filename, lineno); + return false; + } if (nullptr == _cond) { _cond = c; } else {
