Skip to content

Commit

Permalink
Validate redirection location and restrict status codes (#310)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Anilm3
Anilm3 authored Jun 24, 2024
1 parent 15a3588 commit 445c1a8
Show file tree
Hide file tree
Showing 4 changed files with 398 additions and 30 deletions.
18 changes: 17 additions & 1 deletion src/parser/actions_parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "log.hpp"
#include "parser/common.hpp"
#include "parser/parser.hpp"
#include "uri_utils.hpp"

namespace ddwaf::parser::v2 {

Expand All @@ -31,10 +32,25 @@ void validate_and_add_redirect(
return;
}

// Validate the URL;
// - Check it's a valid URL
// - If it has a scheme, check it's http or https
// - If it doesn't have a scheme:
// - Check it also doesn't have an authority
// - Check it's a path starting with /
auto decomposed = uri_parse(it->second);
if (!decomposed.has_value() ||
(!decomposed->scheme.empty() && decomposed->scheme != "http" &&
decomposed->scheme != "https") ||
(decomposed->scheme_and_authority.empty() && !decomposed->path.starts_with('/'))) {
builder.alias_default_action_to("block", id);
return;
}

it = parameters.find("status_code");
if (it != parameters.end()) {
auto [res, code] = ddwaf::from_string<unsigned>(it->second);
if (!res || code < 300 || code > 399) {
if (!res || (code != 301 && code != 302 && code != 303 && code != 307)) {
it->second = "303";
}
} else {
Expand Down
41 changes: 22 additions & 19 deletions src/uri_utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
/ path-absolute
/ path-rootless
/ path-empty
relative-ref = relative-part [ "?" query ] [ "#" fragment ]
relative-part = "//" authority path-abempty
/ path-absolute
/ path-noscheme -> Not supported
/ path-empty -> Not supported
scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
authority = [ userinfo "@" ] host [ ":" port ]
userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
Expand Down Expand Up @@ -59,6 +64,7 @@ namespace {
enum class token_type {
none,
scheme,
scheme_authority_or_path,
hierarchical_part,
authority,
userinfo,
Expand All @@ -67,7 +73,6 @@ enum class token_type {
ipv6address,
regname_or_ipv4address,
path,
path_no_authority,
query,
fragment,
};
Expand Down Expand Up @@ -107,7 +112,7 @@ std::optional<uri_decomposed> uri_parse(std::string_view uri)
uri_decomposed decomposed;
decomposed.raw = uri;

auto expected_token = token_type::scheme;
auto expected_token = token_type::scheme_authority_or_path;
auto lookahead_token = token_type::none;

// Authority helpers
Expand All @@ -120,6 +125,20 @@ std::optional<uri_decomposed> uri_parse(std::string_view uri)
expected_token = token_type::none;

switch (current_token) {
case token_type::scheme_authority_or_path: {
if (uri[i] == '/') {
// Path or authority
if ((i + 1) < uri.size() && uri[i + 1] == '/') {
expected_token = token_type::authority;
i += 2;
} else {
expected_token = token_type::path;
}
} else if (isalpha(uri[i])) {
expected_token = token_type::scheme;
}
break;
}
case token_type::scheme: {
auto token_begin = i;
if (!isalpha(uri[i++])) {
Expand Down Expand Up @@ -157,26 +176,10 @@ std::optional<uri_decomposed> uri_parse(std::string_view uri)
i += 2;
} else {
// Otherwise we expect a path (path-absolute, path-rootless, path-empty)
expected_token = token_type::path_no_authority;
expected_token = token_type::path;
}
break;
}
case token_type::path_no_authority: {
auto token_begin = i;
// The path can be empty but we wouldn't be here...
while (i < uri.size()) {
const auto c = uri[i++];
if (!is_path_char(c) && c != '/') {
return std::nullopt;
}
}

decomposed.path_index = token_begin;
decomposed.path = uri.substr(token_begin, i - token_begin);

// We're done, nothing else to parse
return decomposed;
}
case token_type::authority: {
auto token_begin = i;
authority_end = uri.find_first_of("/?#", i);
Expand Down
177 changes: 173 additions & 4 deletions tests/parser_v2_actions_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2021 Datadog, Inc.

#include "fmt/core.h"
#include "parser/common.hpp"
#include "parser/parser.hpp"
#include "test_utils.hpp"
Expand Down Expand Up @@ -89,9 +90,79 @@ TEST(TestParserV2Actions, SingleAction)
}

TEST(TestParserV2Actions, RedirectAction)
{
std::vector<std::tuple<std::string, std::string, std::string>> redirections{
{"redirect_301", "301", "http://www.datadoghq.com"},
{"redirect_302", "302", "http://www.datadoghq.com"},
{"redirect_303", "303", "http://www.datadoghq.com"},
{"redirect_307", "307", "http://www.datadoghq.com"},
{"redirect_https", "303", "https://www.datadoghq.com"},
{"redirect_path", "303", "/security/appsec"},
};

std::string yaml;
yaml.append("[");
for (auto &[name, status_code, url] : redirections) {
yaml += fmt::format("{{id: {}, parameters: {{location: \"{}\", status_code: {}}}, type: "
"redirect_request}},",
name, url, status_code);
}
yaml.append("]");
auto object = yaml_to_object(yaml);

ddwaf::ruleset_info::section_info section;
auto actions_array = static_cast<parameter::vector>(parameter(object));
auto actions = parser::v2::parse_actions(actions_array, section);
ddwaf_object_free(&object);

{
ddwaf::parameter root;
section.to_object(root);

auto root_map = static_cast<parameter::map>(root);

auto loaded = ddwaf::parser::at<parameter::string_set>(root_map, "loaded");
EXPECT_EQ(loaded.size(), 6);
EXPECT_NE(loaded.find("redirect_301"), loaded.end());
EXPECT_NE(loaded.find("redirect_302"), loaded.end());
EXPECT_NE(loaded.find("redirect_303"), loaded.end());
EXPECT_NE(loaded.find("redirect_307"), loaded.end());
EXPECT_NE(loaded.find("redirect_https"), loaded.end());
EXPECT_NE(loaded.find("redirect_path"), loaded.end());

auto failed = ddwaf::parser::at<parameter::string_set>(root_map, "failed");
EXPECT_EQ(failed.size(), 0);

auto errors = ddwaf::parser::at<parameter::map>(root_map, "errors");
EXPECT_EQ(errors.size(), 0);

ddwaf_object_free(&root);
}

EXPECT_EQ(actions->size(), 10);
EXPECT_TRUE(actions->contains("block"));
EXPECT_TRUE(actions->contains("stack_trace"));
EXPECT_TRUE(actions->contains("extract_schema"));
EXPECT_TRUE(actions->contains("monitor"));

for (auto &[name, status_code, url] : redirections) {
ASSERT_TRUE(actions->contains(name));

const auto &spec = actions->at(name);
EXPECT_EQ(spec.type, action_type::redirect_request) << name;
EXPECT_EQ(spec.type_str, "redirect_request");
EXPECT_EQ(spec.parameters.size(), 2);

const auto &parameters = spec.parameters;
EXPECT_STR(parameters.at("status_code"), status_code.c_str());
EXPECT_STR(parameters.at("location"), url.c_str());
}
}

TEST(TestParserV2Actions, RedirectActionInvalidStatusCode)
{
auto object = yaml_to_object(
R"([{id: redirect, parameters: {location: "http://www.google.com", status_code: 302}, type: redirect_request}])");
R"([{id: redirect, parameters: {location: "http://www.google.com", status_code: 404}, type: redirect_request}])");

ddwaf::ruleset_info::section_info section;
auto actions_array = static_cast<parameter::vector>(parameter(object));
Expand Down Expand Up @@ -131,15 +202,15 @@ TEST(TestParserV2Actions, RedirectAction)
EXPECT_EQ(spec.parameters.size(), 2);

const auto &parameters = spec.parameters;
EXPECT_STR(parameters.at("status_code"), "302");
EXPECT_STR(parameters.at("status_code"), "303");
EXPECT_STR(parameters.at("location"), "http://www.google.com");
}
}

TEST(TestParserV2Actions, RedirectActionInvalidStatusCode)
TEST(TestParserV2Actions, RedirectActionInvalid300StatusCode)
{
auto object = yaml_to_object(
R"([{id: redirect, parameters: {location: "http://www.google.com", status_code: 404}, type: redirect_request}])");
R"([{id: redirect, parameters: {location: "http://www.google.com", status_code: 304}, type: redirect_request}])");

ddwaf::ruleset_info::section_info section;
auto actions_array = static_cast<parameter::vector>(parameter(object));
Expand Down Expand Up @@ -281,6 +352,104 @@ TEST(TestParserV2Actions, RedirectActionMissingLocation)
}
}

TEST(TestParserV2Actions, RedirectActionNonHttpURL)
{
auto object = yaml_to_object(
R"([{id: redirect, parameters: {status_code: 303, location: ftp://myftp.mydomain.com}, type: redirect_request}])");

ddwaf::ruleset_info::section_info section;
auto actions_array = static_cast<parameter::vector>(parameter(object));
auto actions = parser::v2::parse_actions(actions_array, section);
ddwaf_object_free(&object);

{
ddwaf::parameter root;
section.to_object(root);

auto root_map = static_cast<parameter::map>(root);

auto loaded = ddwaf::parser::at<parameter::string_set>(root_map, "loaded");
EXPECT_EQ(loaded.size(), 1);
EXPECT_NE(loaded.find("redirect"), loaded.end());

auto failed = ddwaf::parser::at<parameter::string_set>(root_map, "failed");
EXPECT_EQ(failed.size(), 0);

auto errors = ddwaf::parser::at<parameter::map>(root_map, "errors");
EXPECT_EQ(errors.size(), 0);

ddwaf_object_free(&root);
}

EXPECT_EQ(actions->size(), 5);
EXPECT_TRUE(actions->contains("redirect"));
EXPECT_TRUE(actions->contains("block"));
EXPECT_TRUE(actions->contains("stack_trace"));
EXPECT_TRUE(actions->contains("extract_schema"));
EXPECT_TRUE(actions->contains("monitor"));

{
const auto &spec = actions->at("redirect");
EXPECT_EQ(spec.type, action_type::block_request);
EXPECT_EQ(spec.type_str, "block_request");
EXPECT_EQ(spec.parameters.size(), 3);

const auto &parameters = spec.parameters;
EXPECT_STR(parameters.at("status_code"), "403");
EXPECT_STR(parameters.at("grpc_status_code"), "10");
EXPECT_STR(parameters.at("type"), "auto");
}
}

TEST(TestParserV2Actions, RedirectActionInvalidRelativePathURL)
{
auto object = yaml_to_object(
R"([{id: redirect, parameters: {status_code: 303, location: ../../../etc/passwd}, type: redirect_request}])");

ddwaf::ruleset_info::section_info section;
auto actions_array = static_cast<parameter::vector>(parameter(object));
auto actions = parser::v2::parse_actions(actions_array, section);
ddwaf_object_free(&object);

{
ddwaf::parameter root;
section.to_object(root);

auto root_map = static_cast<parameter::map>(root);

auto loaded = ddwaf::parser::at<parameter::string_set>(root_map, "loaded");
EXPECT_EQ(loaded.size(), 1);
EXPECT_NE(loaded.find("redirect"), loaded.end());

auto failed = ddwaf::parser::at<parameter::string_set>(root_map, "failed");
EXPECT_EQ(failed.size(), 0);

auto errors = ddwaf::parser::at<parameter::map>(root_map, "errors");
EXPECT_EQ(errors.size(), 0);

ddwaf_object_free(&root);
}

EXPECT_EQ(actions->size(), 5);
EXPECT_TRUE(actions->contains("redirect"));
EXPECT_TRUE(actions->contains("block"));
EXPECT_TRUE(actions->contains("stack_trace"));
EXPECT_TRUE(actions->contains("extract_schema"));
EXPECT_TRUE(actions->contains("monitor"));

{
const auto &spec = actions->at("redirect");
EXPECT_EQ(spec.type, action_type::block_request);
EXPECT_EQ(spec.type_str, "block_request");
EXPECT_EQ(spec.parameters.size(), 3);

const auto &parameters = spec.parameters;
EXPECT_STR(parameters.at("status_code"), "403");
EXPECT_STR(parameters.at("grpc_status_code"), "10");
EXPECT_STR(parameters.at("type"), "auto");
}
}

TEST(TestParserV2Actions, OverrideDefaultBlockAction)
{
auto object = yaml_to_object(
Expand Down
Loading

0 comments on commit 445c1a8

Please sign in to comment.