Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Negated scalar condition for matchers #335

Merged
merged 29 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ec70644
Extend exists operator to support key paths and negation
Anilm3 Aug 15, 2024
f6a656d
Add tests
Anilm3 Aug 15, 2024
039d73d
More tests
Anilm3 Aug 16, 2024
758ce05
More tests and improvements
Anilm3 Aug 16, 2024
9ffd7df
Add exists integration tests
Anilm3 Aug 16, 2024
7e03b50
Simplify parser
Anilm3 Aug 18, 2024
e6dc7c3
Merge branch 'master' into anilm3/negated_exists
Anilm3 Aug 19, 2024
c67ce49
Merge branch 'master' into anilm3/negated_exists
Anilm3 Aug 21, 2024
ba427a7
Lint fixes
Anilm3 Aug 21, 2024
e602c9a
Merge branch 'master' into anilm3/negated_exists
Anilm3 Sep 13, 2024
a5d8f5d
Address review comment
Anilm3 Sep 18, 2024
b72db05
Negated scalar condition for matchers
Anilm3 Aug 16, 2024
d1ccef4
More tests
Anilm3 Aug 16, 2024
4f28a1c
Support multiple types per matcher
Anilm3 Sep 11, 2024
4a1c05b
Refactor matcher parser
Anilm3 Sep 11, 2024
ca28b93
Reduce set of negated matchers
Anilm3 Sep 11, 2024
42fc3fb
Format and lint
Anilm3 Sep 12, 2024
42af9eb
Merge branch 'master' into anilm3/negated_scalar_condition
Anilm3 Sep 18, 2024
4013b37
More tests
Anilm3 Sep 19, 2024
bd5bd2f
Fix format
Anilm3 Sep 19, 2024
600e66a
Add test cases
Anilm3 Sep 19, 2024
4ff6ada
Fixes and more tests
Anilm3 Sep 20, 2024
494cfaf
Small fix
Anilm3 Sep 20, 2024
afe19e8
Small extra test cases
Anilm3 Sep 20, 2024
4e16886
Split booleans and integer on equals
Anilm3 Sep 20, 2024
60b8e09
More tests
Anilm3 Sep 21, 2024
b7fc7a5
Timeout tests and missing free
Anilm3 Sep 21, 2024
5db918a
Add requires to eval_object/target
Anilm3 Oct 2, 2024
1416eae
Simplify condition parsing and unify negated matcher names
Anilm3 Oct 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
# readability-function-cognitive-complexity temporarily disabled until clang-tidy is fixed
# right now emalloc causes it to misbehave
Checks: '*,misc-const-correctness,-bugprone-reserved-identifier,-hicpp-signed-bitwise,-llvmlibc-restrict-system-libc-headers,-altera-unroll-loops,-hicpp-named-parameter,-cert-dcl37-c,-cert-dcl51-cpp,-read,-cppcoreguidelines-init-variables,-cppcoreguidelines-avoid-non-const-global-variables,-altera-id-dependent-backward-branch,-performance-no-int-to-ptr,-altera-struct-pack-align,-google-readability-casting,-modernize-use-trailing-return-type,-llvmlibc-implementation-in-namespace,-llvmlibc-callee-namespace,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-fuchsia-default-arguments-declarations,-fuchsia-overloaded-operator,-cppcoreguidelines-pro-type-union-access,-fuchsia-default-arguments-calls,-cppcoreguidelines-non-private-member-variables-in-classes,-misc-non-private-member-variables-in-classes,-google-readability-todo,-llvm-header-guard,-readability-function-cognitive-complexity,-readability-identifier-length,-cppcoreguidelines-owning-memory,-cert-err58-cpp,-fuchsia-statically-constructed-objects,-google-build-using-namespace,-hicpp-avoid-goto,-cppcoreguidelines-avoid-goto,-hicpp-no-array-decay,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-avoid-magic-numbers,-readability-magic-numbers,-abseil-string-find-str-contains,-bugprone-unchecked-optional-access,-readability-use-anyofallof,-modernize-loop-convert,-cppcoreguidelines-avoid-c-arrays,-hicpp-avoid-c-arrays,-cppcoreguidelines-no-malloc'
Checks: '*,misc-const-correctness,-bugprone-reserved-identifier,-hicpp-signed-bitwise,-llvmlibc-restrict-system-libc-headers,-altera-unroll-loops,-hicpp-named-parameter,-cert-dcl37-c,-cert-dcl51-cpp,-read,-cppcoreguidelines-init-variables,-cppcoreguidelines-avoid-non-const-global-variables,-altera-id-dependent-backward-branch,-performance-no-int-to-ptr,-altera-struct-pack-align,-google-readability-casting,-modernize-use-trailing-return-type,-llvmlibc-implementation-in-namespace,-llvmlibc-callee-namespace,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-fuchsia-default-arguments-declarations,-fuchsia-overloaded-operator,-cppcoreguidelines-pro-type-union-access,-fuchsia-default-arguments-calls,-cppcoreguidelines-non-private-member-variables-in-classes,-misc-non-private-member-variables-in-classes,-google-readability-todo,-llvm-header-guard,-readability-function-cognitive-complexity,-readability-identifier-length,-cppcoreguidelines-owning-memory,-cert-err58-cpp,-fuchsia-statically-constructed-objects,-google-build-using-namespace,-hicpp-avoid-goto,-cppcoreguidelines-avoid-goto,-hicpp-no-array-decay,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-avoid-magic-numbers,-readability-magic-numbers,-abseil-string-find-str-contains,-bugprone-unchecked-optional-access,-readability-use-anyofallof,-modernize-loop-convert,-cppcoreguidelines-avoid-c-arrays,-hicpp-avoid-c-arrays,-cppcoreguidelines-no-malloc,-llvmlibc-inline-function-decl'
WarningsAsErrors: '*'
HeaderFilterRegex: ''
CheckOptions:
Expand Down
108 changes: 85 additions & 23 deletions src/condition/scalar_condition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ namespace ddwaf {

namespace {

template <typename Iterator>
std::optional<condition_match> eval_object(Iterator &it, std::string_view address, bool ephemeral,
template <typename ResultType, typename Iterator>
ResultType eval_object(Iterator &it, std::string_view address, bool ephemeral,
const matcher::base &matcher, const std::span<const transformer_id> &transformers,
const object_limits &limits)
requires(std::is_same_v<ResultType, bool> ||
std::is_same_v<ResultType, std::optional<condition_match>>)
{
// The iterator is guaranteed to be valid at this point, which means the
// object pointer should not be nullptr
Expand All @@ -60,8 +62,12 @@ std::optional<condition_match> eval_object(Iterator &it, std::string_view addres

DDWAF_TRACE("Target {} matched parameter value {}", address, highlight);

return {{{{"input"sv, object_to_string(dst), address, it.get_current_path()}},
{std::move(highlight)}, matcher.name(), matcher.to_string(), ephemeral}};
if constexpr (std::is_same_v<ResultType, bool>) {
return true;
} else {
return {{{{"input"sv, object_to_string(dst), address, it.get_current_path()}},
Anilm3 marked this conversation as resolved.
Show resolved Hide resolved
{std::move(highlight)}, matcher.name(), matcher.to_string(), ephemeral}};
}
}
}
}
Expand All @@ -73,26 +79,32 @@ std::optional<condition_match> eval_object(Iterator &it, std::string_view addres

DDWAF_TRACE("Target {} matched parameter value {}", address, highlight);

return {{{{"input"sv, object_to_string(src), address, it.get_current_path()}},
{std::move(highlight)}, matcher.name(), matcher.to_string(), ephemeral}};
if constexpr (std::is_same_v<ResultType, bool>) {
return true;
} else {
return {{{{"input"sv, object_to_string(src), address, it.get_current_path()}},
{std::move(highlight)}, matcher.name(), matcher.to_string(), ephemeral}};
}
}

template <typename Iterator>
std::optional<condition_match> eval_target(Iterator &it, std::string_view address, bool ephemeral,
template <typename ResultType, typename Iterator>
ResultType eval_target(Iterator &it, std::string_view address, bool ephemeral,
const matcher::base &matcher, const std::span<const transformer_id> &transformers,
const object_limits &limits, ddwaf::timer &deadline)
requires(std::is_same_v<ResultType, bool> ||
std::is_same_v<ResultType, std::optional<condition_match>>)
{
for (; it; ++it) {
if (deadline.expired()) {
throw ddwaf::timeout_exception();
}

if (it.type() != matcher.supported_type()) {
if (!matcher.is_supported_type(it.type())) {
continue;
}

auto match = eval_object(it, address, ephemeral, matcher, transformers, limits);
if (match.has_value()) {
auto match = eval_object<ResultType>(it, address, ephemeral, matcher, transformers, limits);
if (match) {
// If this target matched, we can stop processing
return match;
}
Expand All @@ -101,29 +113,30 @@ std::optional<condition_match> eval_target(Iterator &it, std::string_view addres
return {};
}

} // namespace

const matcher::base *scalar_condition::get_matcher(
const std::unordered_map<std::string, std::shared_ptr<matcher::base>> &dynamic_matchers) const
const matcher::base *get_matcher(const std::unique_ptr<matcher::base> &matcher,
const std::string &data_id,
const std::unordered_map<std::string, std::shared_ptr<matcher::base>> &dynamic_matchers)
{
if (matcher_ || data_id_.empty()) {
return matcher_.get();
if (matcher || data_id.empty()) {
return matcher.get();
}

auto it = dynamic_matchers.find(data_id_);
auto it = dynamic_matchers.find(data_id);
if (it != dynamic_matchers.end()) {
return it->second.get();
}

return nullptr;
}

} // namespace

eval_result scalar_condition::eval(condition_cache &cache, const object_store &store,
const exclusion::object_set_ref &objects_excluded,
const std::unordered_map<std::string, std::shared_ptr<matcher::base>> &dynamic_matchers,
ddwaf::timer &deadline) const
{
const auto *matcher = get_matcher(dynamic_matchers);
const auto *matcher = get_matcher(matcher_, data_id_, dynamic_matchers);
if (matcher == nullptr) {
return {};
}
Expand Down Expand Up @@ -152,21 +165,70 @@ eval_result scalar_condition::eval(condition_cache &cache, const object_store &s
// TODO: iterators could be cached to avoid reinitialisation
if (target.source == data_source::keys) {
object::key_iterator it(object, target.key_path, objects_excluded, limits_);
match = eval_target(
match = eval_target<std::optional<condition_match>>(
it, target.name, ephemeral, *matcher, target.transformers, limits_, deadline);
} else {
object::value_iterator it(object, target.key_path, objects_excluded, limits_);
match = eval_target(
match = eval_target<std::optional<condition_match>>(
it, target.name, ephemeral, *matcher, target.transformers, limits_, deadline);
}

if (match.has_value()) {
cache.match = std::move(match);
return {true, ephemeral};
return {.outcome = true, .ephemeral = ephemeral};
}
}

return {false, false};
return {.outcome = false, .ephemeral = false};
}

eval_result scalar_negated_condition::eval(condition_cache &cache, const object_store &store,
const exclusion::object_set_ref &objects_excluded,
const std::unordered_map<std::string, std::shared_ptr<matcher::base>> &dynamic_matchers,
ddwaf::timer &deadline) const
{
if (deadline.expired()) {
throw ddwaf::timeout_exception();
}

const auto *matcher = get_matcher(matcher_, data_id_, dynamic_matchers);
if (matcher == nullptr) {
return {};
}

if (cache.targets.size() != 1) {
cache.targets.assign(1, nullptr);
}

auto [object, attr] = store.get_target(target_.index);
if (object == nullptr || object == cache.targets[0]) {
return {};
}

const bool ephemeral = (attr == object_store::attribute::ephemeral);
if (!ephemeral) {
cache.targets[0] = object;
}

bool match = false;
if (target_.source == data_source::keys) {
object::key_iterator it(object, target_.key_path, objects_excluded, limits_);
match = eval_target<bool>(
it, target_.name, ephemeral, *matcher, target_.transformers, limits_, deadline);
} else {
object::value_iterator it(object, target_.key_path, objects_excluded, limits_);
match = eval_target<bool>(
it, target_.name, ephemeral, *matcher, target_.transformers, limits_, deadline);
}

if (!match) {
cache.match = {{{{"input"sv, object_to_string(*object), target_.name,
{target_.key_path.begin(), target_.key_path.end()}}},
{}, matcher->negated_name(), matcher->to_string(), ephemeral}};
return {.outcome = true, .ephemeral = ephemeral};
}

return {.outcome = false, .ephemeral = false};
}

} // namespace ddwaf
57 changes: 49 additions & 8 deletions src/condition/scalar_condition.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ class scalar_condition : public base_condition {
: matcher_(std::move(matcher)), data_id_(std::move(data_id)), limits_(limits)
{
if (args.size() > 1) {
throw std::invalid_argument("Matcher initialised with more than one argument");
throw std::invalid_argument("matcher initialised with more than one argument");
}

if (args.empty()) {
throw std::invalid_argument("Matcher initialised without arguments");
throw std::invalid_argument("matcher initialised without arguments");
}

targets_ = std::move(args[0].targets);
Expand All @@ -39,18 +39,59 @@ class scalar_condition : public base_condition {

static constexpr auto arguments()
{
return std::array<parameter_specification, 1>{{{"inputs", true, false}}};
return std::array<parameter_specification, 1>{
{{.name = "inputs", .variadic = true, .optional = false}}};
}

protected:
[[nodiscard]] const matcher::base *get_matcher(
const std::unordered_map<std::string, std::shared_ptr<matcher::base>> &dynamic_matchers)
const;

std::unique_ptr<matcher::base> matcher_;
std::string data_id_;
std::vector<condition_target> targets_;
const object_limits limits_;
object_limits limits_;
};

class scalar_negated_condition : public base_condition {
public:
scalar_negated_condition(std::unique_ptr<matcher::base> &&matcher, std::string data_id,
std::vector<condition_parameter> args, const object_limits &limits = {})
: matcher_(std::move(matcher)), data_id_(std::move(data_id)), limits_(limits)
{
if (args.size() > 1) {
throw std::invalid_argument("matcher initialised with more than one argument");
}

if (args.empty()) {
throw std::invalid_argument("matcher initialised without arguments");
}

if (args[0].targets.size() > 1) {
throw std::invalid_argument("negated matchers don't support variadic arguments");
}

target_ = std::move(args[0].targets[0]);
}

eval_result eval(condition_cache &cache, const object_store &store,
const exclusion::object_set_ref &objects_excluded,
const std::unordered_map<std::string, std::shared_ptr<matcher::base>> &dynamic_matchers,
ddwaf::timer &deadline) const override;

void get_addresses(std::unordered_map<target_index, std::string> &addresses) const override
{
addresses.emplace(target_.index, target_.name);
}

static constexpr auto arguments()
{
return std::array<parameter_specification, 1>{
{{.name = "inputs", .variadic = false, .optional = false}}};
}

protected:
std::unique_ptr<matcher::base> matcher_;
std::string data_id_;
condition_target target_;
object_limits limits_;
};

} // namespace ddwaf
20 changes: 11 additions & 9 deletions src/matcher/base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,13 @@ class base {
// for example, through a constexpr class static string_view initialised
// with a literal.
[[nodiscard]] virtual std::string_view name() const = 0;
[[nodiscard]] virtual std::string_view negated_name() const = 0;
// Returns a string representing this particular instance of the operator, for example,
// an operator matching regexes could provide the regex as its string representation.
[[nodiscard]] virtual std::string_view to_string() const = 0;

// Scalar matcher methods
[[nodiscard]] virtual DDWAF_OBJ_TYPE supported_type() const = 0;
[[nodiscard]] virtual bool is_supported_type(DDWAF_OBJ_TYPE type) const = 0;

[[nodiscard]] virtual std::pair<bool, std::string> match(const ddwaf_object &obj) const = 0;
};
Expand All @@ -47,16 +48,17 @@ template <typename T> class base_impl : public base {
base_impl &operator=(const base_impl &) = default;
base_impl &operator=(base_impl &&) noexcept = default;

[[nodiscard]] std::string_view name() const override { return T::name_impl(); }
[[nodiscard]] std::string_view name() const override { return T::matcher_name; }
[[nodiscard]] std::string_view negated_name() const override { return T::negated_matcher_name; }

[[nodiscard]] std::string_view to_string() const override
{
return static_cast<const T *>(this)->to_string_impl();
}

[[nodiscard]] DDWAF_OBJ_TYPE supported_type() const override
[[nodiscard]] bool is_supported_type(DDWAF_OBJ_TYPE type) const override
{
return T::supported_type_impl();
return T::is_supported_type_impl(type);
}

// Helper used for testing purposes
Expand All @@ -68,31 +70,31 @@ template <typename T> class base_impl : public base {
[[nodiscard]] std::pair<bool, std::string> match(const ddwaf_object &obj) const override
{
const auto *ptr = static_cast<const T *>(this);
if constexpr (T::supported_type_impl() == DDWAF_OBJ_STRING) {
if constexpr (T::is_supported_type_impl(DDWAF_OBJ_STRING)) {
if (obj.type == DDWAF_OBJ_STRING && obj.stringValue != nullptr) {
return ptr->match_impl({obj.stringValue, static_cast<std::size_t>(obj.nbEntries)});
}
}

if constexpr (T::supported_type_impl() == DDWAF_OBJ_SIGNED) {
if constexpr (T::is_supported_type_impl(DDWAF_OBJ_SIGNED)) {
if (obj.type == DDWAF_OBJ_SIGNED) {
return ptr->match_impl(obj.intValue);
}
}

if constexpr (T::supported_type_impl() == DDWAF_OBJ_UNSIGNED) {
if constexpr (T::is_supported_type_impl(DDWAF_OBJ_UNSIGNED)) {
if (obj.type == DDWAF_OBJ_UNSIGNED) {
return ptr->match_impl(obj.uintValue);
}
}

if constexpr (T::supported_type_impl() == DDWAF_OBJ_BOOL) {
if constexpr (T::is_supported_type_impl(DDWAF_OBJ_BOOL)) {
if (obj.type == DDWAF_OBJ_BOOL) {
return ptr->match_impl(obj.boolean);
}
}

if constexpr (T::supported_type_impl() == DDWAF_OBJ_FLOAT) {
if constexpr (T::is_supported_type_impl(DDWAF_OBJ_FLOAT)) {
if (obj.type == DDWAF_OBJ_FLOAT) {
return ptr->match_impl(obj.f64);
}
Expand Down
Loading
Loading