Skip to content

Commit 4af94ea

Browse files
committed
Fix NoSQL object comparison being too strict
1 parent 24bb970 commit 4af94ea

2 files changed

Lines changed: 117 additions & 1 deletion

File tree

aikido_zen/vulnerabilities/nosql_injection/__init__.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def match_filter_part_in_user(user_input, filter_part, path_to_payload=None):
2424

2525
if is_mapping(user_input):
2626
filtered_input = remove_keys_that_dont_start_with_dollar_sign(user_input)
27-
if filtered_input == filter_part:
27+
if is_user_operators_subset_of(filtered_input, filter_part):
2828
return {
2929
"match": True,
3030
"pathToPayload": build_path_to_payload(path_to_payload),
@@ -57,6 +57,20 @@ def remove_keys_that_dont_start_with_dollar_sign(nosql_filter):
5757
return {key: value for key, value in nosql_filter.items() if key.startswith("$")}
5858

5959

60+
def is_user_operators_subset_of(user_operators, filter_operators):
61+
"""
62+
Returns True if every operator in user_operators is present in filter_operators
63+
with the same value — i.e. the user-supplied operators are a subset of the filter.
64+
An empty user_operators dict never matches (no operators = no injection).
65+
"""
66+
has_keys = False
67+
for key, value in user_operators.items():
68+
if key not in filter_operators or filter_operators[key] != value:
69+
return False
70+
has_keys = True
71+
return has_keys
72+
73+
6074
def find_filter_part_with_operators(user_input, part_of_filter):
6175
"""
6276
This looks for parts in the filter that have NSQL operators (e.g. $)

aikido_zen/vulnerabilities/nosql_injection/init_test.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,3 +540,105 @@ def test_ignores_safe_pipeline_aggregations(create_context):
540540
)
541541
== {}
542542
)
543+
544+
545+
def test_detects_injection_when_app_merges_user_operators_with_own_dollar_keys(
546+
create_context,
547+
):
548+
assert detect_nosql_injection(
549+
create_context(body={"username": {"$ne": None}}),
550+
{"title": {"$ne": None, "$exists": True}},
551+
) == {
552+
"injection": True,
553+
"source": "body",
554+
"pathToPayload": ".username",
555+
"payload": {"$ne": None, "$exists": True},
556+
}
557+
558+
559+
def test_detects_injection_when_app_prepends_own_dollar_key_before_user_operators(
560+
create_context,
561+
):
562+
assert detect_nosql_injection(
563+
create_context(body={"username": {"$gt": ""}}),
564+
{"title": {"$exists": True, "$gt": ""}},
565+
) == {
566+
"injection": True,
567+
"source": "body",
568+
"pathToPayload": ".username",
569+
"payload": {"$exists": True, "$gt": ""},
570+
}
571+
572+
573+
def test_does_not_flag_when_user_operator_value_differs(create_context):
574+
assert (
575+
detect_nosql_injection(
576+
create_context(body={"username": {"$ne": "different"}}),
577+
{"title": {"$ne": None, "$exists": True}},
578+
)
579+
== {}
580+
)
581+
582+
583+
def test_detects_injection_when_app_adds_non_dollar_key_to_subobject(create_context):
584+
assert detect_nosql_injection(
585+
create_context(body={"field": {"$elemMatch": {"$gt": 5}}}),
586+
{"items": {"$elemMatch": {"$gt": 5, "verified": True}}},
587+
) == {
588+
"injection": True,
589+
"source": "body",
590+
"pathToPayload": ".field.$elemMatch",
591+
"payload": {"$gt": 5},
592+
}
593+
594+
595+
def test_does_not_flag_when_user_sends_empty_object(create_context):
596+
assert (
597+
detect_nosql_injection(
598+
create_context(body={"username": {}}),
599+
{"title": {"$exists": True}},
600+
)
601+
== {}
602+
)
603+
604+
605+
def test_does_not_flag_when_user_object_has_only_non_dollar_keys(create_context):
606+
assert (
607+
detect_nosql_injection(
608+
create_context(body={"username": {"name": "alice"}}),
609+
{"title": {"$exists": True}},
610+
)
611+
== {}
612+
)
613+
614+
615+
def test_does_not_flag_when_user_dollar_key_absent_from_filter(create_context):
616+
assert (
617+
detect_nosql_injection(
618+
create_context(body={"username": {"$ne": None}}),
619+
{"title": {"$exists": True}},
620+
)
621+
== {}
622+
)
623+
624+
625+
def test_does_not_flag_when_filter_uses_subset_of_user_operators(create_context):
626+
assert (
627+
detect_nosql_injection(
628+
create_context(body={"username": {"$ne": None, "$gt": 0}}),
629+
{"title": {"$ne": None}},
630+
)
631+
== {}
632+
)
633+
634+
635+
def test_does_not_flag_when_app_ignores_user_operators_and_uses_hardcoded_filter(
636+
create_context,
637+
):
638+
assert (
639+
detect_nosql_injection(
640+
create_context(body={"username": {"$ne": None}}),
641+
{"username": "hardcoded"},
642+
)
643+
== {}
644+
)

0 commit comments

Comments
 (0)