diff --git a/aikido_zen/helpers/extract_data_from_xml_body.py b/aikido_zen/helpers/extract_data_from_xml_body.py index 670816f6a..33fe0307a 100644 --- a/aikido_zen/helpers/extract_data_from_xml_body.py +++ b/aikido_zen/helpers/extract_data_from_xml_body.py @@ -5,7 +5,7 @@ def extract_data_from_xml_body(user_input, root_element): - """Extracts all attributes from the xml and adds them to context""" + """Extracts all attributes and text from the xml and adds them to context""" try: context = ctx.get_current_context() if not context or not isinstance(context.body, str): @@ -16,12 +16,24 @@ def extract_data_from_xml_body(user_input, root_element): if user_input != context.body: return - extracted_xml_attrs = context.xml - for el in root_element: - for k, v in el.items(): - if not extracted_xml_attrs.get(k): - extracted_xml_attrs[k] = set() - extracted_xml_attrs[k].add(v) + extracted_xml = context.xml + tag = None + for element in root_element.iter(): + if isinstance(element.tag, str): + tag = element.tag + texts = (element.text, element.tail) + for key, value in element.items(): + extracted_xml.setdefault(key, set()).add(value) + else: + # The text after a comment or processing instruction is in its tail. + texts = (element.tail,) if tag else () + for text in texts: + if not text: + continue + stripped = text.strip() + if not stripped: + continue + extracted_xml.setdefault(tag, set()).update((text, stripped)) context.set_as_current_context() except Exception as e: logger.debug("Exception occurred when extracting XML: %s", e) diff --git a/aikido_zen/helpers/extract_data_from_xml_body_test.py b/aikido_zen/helpers/extract_data_from_xml_body_test.py index f17a5f379..a99e2c16b 100644 --- a/aikido_zen/helpers/extract_data_from_xml_body_test.py +++ b/aikido_zen/helpers/extract_data_from_xml_body_test.py @@ -1,5 +1,6 @@ import pytest from unittest.mock import MagicMock, patch +import xml.etree.ElementTree as ET import aikido_zen.context as ctx from .extract_data_from_xml_body import ( extract_data_from_xml_body, @@ -17,10 +18,9 @@ def mock_context(): def test_does_not_crash_when_context_none(mock_context): with patch("aikido_zen.context.get_current_context", return_value=None): user_input = "valid_input" - root_element = [ - {"attr1": "value1", "attr2": "value2"}, - {"attr1": "value3", "attr3": "value4"}, - ] + root_element = ET.fromstring( + '' + ) extract_data_from_xml_body(user_input, root_element) @@ -28,10 +28,9 @@ def test_does_not_crash_when_context_none(mock_context): def test_extract_data_from_xml_body_valid_input(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): user_input = "valid_input" - root_element = [ - {"attr1": "value1", "attr2": "value2"}, - {"attr1": "value3", "attr3": "value4"}, - ] + root_element = ET.fromstring( + '' + ) extract_data_from_xml_body(user_input, root_element) @@ -45,7 +44,7 @@ def test_extract_data_from_xml_body_valid_input(mock_context): def test_extract_data_from_xml_body_invalid_user_input(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): user_input = "invalid_input" - root_element = [{"attr1": "value1"}] + root_element = ET.fromstring('') extract_data_from_xml_body(user_input, root_element) @@ -55,7 +54,7 @@ def test_extract_data_from_xml_body_invalid_user_input(mock_context): def test_extract_data_from_xml_body_empty_root_element(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): user_input = "valid_input" - root_element = [] + root_element = ET.fromstring("") extract_data_from_xml_body(user_input, root_element) @@ -66,7 +65,7 @@ def test_extract_data_from_xml_body_non_string_context_body(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): mock_context.body = 123 # Set body to a non-string value user_input = "valid_input" - root_element = [{"attr1": "value1"}] + root_element = ET.fromstring('') extract_data_from_xml_body(user_input, root_element) @@ -76,8 +75,8 @@ def test_extract_data_from_xml_body_non_string_context_body(mock_context): def test_extract_data_from_xml_body_multiple_calls(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): user_input = "valid_input" - root_element1 = [{"attr1": "value1"}] - root_element2 = [{"attr1": "value2"}] + root_element1 = ET.fromstring('') + root_element2 = ET.fromstring('') extract_data_from_xml_body(user_input, root_element1) extract_data_from_xml_body(user_input, root_element2) @@ -88,11 +87,9 @@ def test_extract_data_from_xml_body_multiple_calls(mock_context): def test_extract_data_from_xml_body_duplicate_attributes(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): user_input = "valid_input" - root_element = [ - {"attr1": "value1"}, - {"attr1": "value1"}, # Duplicate - {"attr2": "value2"}, - ] + root_element = ET.fromstring( + '' + ) extract_data_from_xml_body(user_input, root_element) @@ -102,17 +99,111 @@ def test_extract_data_from_xml_body_duplicate_attributes(mock_context): def test_extract_data_from_xml_body_no_attributes(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): user_input = "valid_input" - root_element = [{}] + root_element = ET.fromstring("") extract_data_from_xml_body(user_input, root_element) assert mock_context.xml == {} +def test_extract_data_from_xml_body_element_text(mock_context): + with patch("aikido_zen.context.get_current_context", return_value=mock_context): + user_input = "valid_input" + root_element = ET.fromstring("value1") + + extract_data_from_xml_body(user_input, root_element) + + assert mock_context.xml == {"id": {"value1"}} + + +def test_extract_data_from_xml_body_indented_element_text(mock_context): + with patch("aikido_zen.context.get_current_context", return_value=mock_context): + user_input = "valid_input" + root_element = ET.fromstring("\n value1\n ") + + extract_data_from_xml_body(user_input, root_element) + + assert mock_context.xml == {"id": {"\n value1\n ", "value1"}} + + +def test_extract_data_from_xml_body_text_between_elements(mock_context): + with patch("aikido_zen.context.get_current_context", return_value=mock_context): + user_input = "valid_input" + root_element = ET.fromstring("beforeafter") + + extract_data_from_xml_body(user_input, root_element) + + assert mock_context.xml == {"root": {"before"}, "a": {"after"}} + + +def test_extract_data_from_xml_body_root_attributes(mock_context): + with patch("aikido_zen.context.get_current_context", return_value=mock_context): + user_input = "valid_input" + root_element = ET.fromstring('') + + extract_data_from_xml_body(user_input, root_element) + + assert mock_context.xml == {"attr1": {"value1"}} + + +def test_extract_data_from_xml_body_nested_attributes(mock_context): + with patch("aikido_zen.context.get_current_context", return_value=mock_context): + user_input = "valid_input" + root_element = ET.fromstring('') + + extract_data_from_xml_body(user_input, root_element) + + assert mock_context.xml == {"attr1": {"value1"}} + + +@pytest.mark.parametrize( + "root_element", [None, "not an element", [{"attr1": "value1"}]] +) +def test_extract_data_from_xml_body_invalid_root_element(mock_context, root_element): + with patch("aikido_zen.context.get_current_context", return_value=mock_context): + extract_data_from_xml_body("valid_input", root_element) + + assert mock_context.xml == {} + + +def test_extract_data_from_xml_body_text_after_a_comment(mock_context): + with patch("aikido_zen.context.get_current_context", return_value=mock_context): + parser = ET.XMLParser(target=ET.TreeBuilder(insert_comments=True)) + root_element = ET.fromstring("bobafter", parser) + + extract_data_from_xml_body("valid_input", root_element) + + assert mock_context.xml == {"a": {"bob", "after"}} + + +def test_extract_data_from_xml_body_text_after_a_processing_instruction(mock_context): + with patch("aikido_zen.context.get_current_context", return_value=mock_context): + parser = ET.XMLParser(target=ET.TreeBuilder(insert_pis=True)) + root_element = ET.fromstring("bobafter", parser) + + extract_data_from_xml_body("valid_input", root_element) + + assert mock_context.xml == {"a": {"bob", "after"}} + + +def test_extract_data_from_xml_body_comments_and_processing_instructions(mock_context): + with patch("aikido_zen.context.get_current_context", return_value=mock_context): + parser = ET.XMLParser( + target=ET.TreeBuilder(insert_comments=True, insert_pis=True) + ) + root_element = ET.fromstring( + "value1", parser + ) + + extract_data_from_xml_body("valid_input", root_element) + + assert mock_context.xml == {"a": {"value1"}} + + def test_extract_data_from_xml_body_context_set_as_current(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): user_input = "valid_input" - root_element = [{"attr1": "value1"}] + root_element = ET.fromstring('') extract_data_from_xml_body(user_input, root_element) @@ -122,7 +213,9 @@ def test_extract_data_from_xml_body_context_set_as_current(mock_context): def test_extract_data_from_xml_body_bytes_user_input(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): user_input = b"valid_input" - root_element = [{"attr1": "value1"}, {"attr2": "value2"}] + root_element = ET.fromstring( + '' + ) extract_data_from_xml_body(user_input, root_element) @@ -132,7 +225,7 @@ def test_extract_data_from_xml_body_bytes_user_input(mock_context): def test_extract_data_from_xml_body_bytes_invalid_user_input(mock_context): with patch("aikido_zen.context.get_current_context", return_value=mock_context): user_input = b"invalid_input" - root_element = [{"attr1": "value1"}] + root_element = ET.fromstring('') extract_data_from_xml_body(user_input, root_element) @@ -144,7 +237,7 @@ def test_extract_data_from_xml_body_bytes_invalid_user_input(mock_context): ) def test_extract_data_from_xml_body_byteslike_user_input(mock_context, user_input): with patch("aikido_zen.context.get_current_context", return_value=mock_context): - root_element = [{"attr1": "value1"}] + root_element = ET.fromstring('') extract_data_from_xml_body(user_input, root_element) @@ -152,12 +245,13 @@ def test_extract_data_from_xml_body_byteslike_user_input(mock_context, user_inpu def test_extract_data_from_xml_body_bytes_invalid_utf8_matches_body(): - body = b'' + body = b'1\xff' context = ctx.Context(body=body, source="test") + root_element = ET.fromstring(context.body) context.set_as_current_context() try: - extract_data_from_xml_body(body, [{"id": "1\ufffd"}]) + extract_data_from_xml_body(body, root_element) - assert context.xml == {"id": {"1\ufffd"}} + assert context.xml == {"id": {"1�"}, "item": {"1�"}} finally: ctx.current_context.set(None) diff --git a/aikido_zen/sources/xml_sources/lxml_test.py b/aikido_zen/sources/xml_sources/lxml_test.py index b79005e26..bdab9c853 100644 --- a/aikido_zen/sources/xml_sources/lxml_test.py +++ b/aikido_zen/sources/xml_sources/lxml_test.py @@ -68,9 +68,12 @@ def test_parse_xml_with_set_context(): context = get_current_context() assert context.xml == { "attr": {"chill"}, + "name": {"Test Name"}, + "ok": {"boomer"}, "smth": {"2"}, "smthelse": {"2"}, - "test": {"test1"}, + "test": {"test1", "test2"}, + "value": {"42"}, } @@ -84,7 +87,27 @@ def test_parse_xml_bytes_with_set_context(): context = get_current_context() assert context.xml == { "attr": {"chill"}, + "name": {"Test Name"}, + "ok": {"boomer"}, "smth": {"2"}, "smthelse": {"2"}, - "test": {"test1"}, + "test": {"test1", "test2"}, + "value": {"42"}, } + + +XML_WITH_COMMENT = """ + + Test Name + +""" + + +def test_parse_xml_keeps_the_text_after_a_comment(): + set_context(body=XML_WITH_COMMENT) + root = parse_xml(XML_WITH_COMMENT) + + assert "".join(root.find("name").itertext()) == "Test Name" + + context = get_current_context() + assert context.xml == {"name": {"Test", " Name", "Name"}} diff --git a/aikido_zen/sources/xml_sources/xml_test.py b/aikido_zen/sources/xml_sources/xml_test.py index 55bb92921..75e77d5e9 100644 --- a/aikido_zen/sources/xml_sources/xml_test.py +++ b/aikido_zen/sources/xml_sources/xml_test.py @@ -70,9 +70,12 @@ def test_parse_xml_with_set_context(): context = get_current_context() assert context.xml == { "attr": {"chill"}, + "name": {"Test Name"}, + "ok": {"boomer"}, "smth": {"2"}, "smthelse": {"2"}, - "test": {"test1"}, + "test": {"test1", "test2"}, + "value": {"42"}, } @@ -86,7 +89,10 @@ def test_parse_xml_bytes_with_set_context(): context = get_current_context() assert context.xml == { "attr": {"chill"}, + "name": {"Test Name"}, + "ok": {"boomer"}, "smth": {"2"}, "smthelse": {"2"}, - "test": {"test1"}, + "test": {"test1", "test2"}, + "value": {"42"}, } diff --git a/end2end/flask_postgres_xml_lxml_test.py b/end2end/flask_postgres_xml_lxml_test.py index 4f471b86e..c89b04543 100644 --- a/end2end/flask_postgres_xml_lxml_test.py +++ b/end2end/flask_postgres_xml_lxml_test.py @@ -6,6 +6,8 @@ # e2e tests for flask_postgres sample app post_url_fw = "http://localhost:8092/xml_post_lxml" post_url_nofw = "http://localhost:8093/xml_post_lxml" +text_url_fw = "http://localhost:8092/xml_post_text_lxml" +text_url_nofw = "http://localhost:8093/xml_post_text_lxml" def test_safe_response_with_firewall(): xml_data = '' @@ -25,8 +27,8 @@ def test_dangerous_response_with_firewall(): time.sleep(5) # Wait for attack to be reported events = fetch_events_from_mock("http://localhost:5000") - attacks = filter_on_event_type(events, "detected_attack") - + attacks = [a for a in filter_on_event_type(events, "detected_attack") if a["request"]["route"] == "/xml_post_lxml"] + assert len(attacks) == 1 del attacks[0]["attack"]["stack"] @@ -49,3 +51,33 @@ def test_dangerous_response_without_firewall(): res = requests.post(post_url_nofw, data=xml_data) assert res.status_code == 200 +def test_dangerous_element_text_with_firewall(): + xml_data = 'Malicious dog\', TRUE); --' + res = requests.post(text_url_fw, data=xml_data) + assert res.status_code == 500 + + time.sleep(5) # Wait for attack to be reported + events = fetch_events_from_mock("http://localhost:5000") + attacks = [a for a in filter_on_event_type(events, "detected_attack") if a["request"]["route"] == "/xml_post_text_lxml"] + + assert len(attacks) == 1 + del attacks[0]["attack"]["stack"] + + assert attacks[0]["attack"] == { + "blocked": True, + "kind": "sql_injection", + 'metadata': { + 'dialect': "postgres", + 'sql': "INSERT INTO dogs (dog_name, isAdmin) VALUES ('Malicious dog', TRUE); --', FALSE)" + }, + 'operation': "psycopg2.Connection.Cursor.execute", + 'pathToPayload': ".dog.[0]", + 'payload': "\"Malicious dog', TRUE); --\"", + 'source': "xml", + 'user': None + } + +def test_dangerous_element_text_without_firewall(): + xml_data = 'Malicious dog\', TRUE); --' + res = requests.post(text_url_nofw, data=xml_data) + assert res.status_code == 200 diff --git a/end2end/flask_postgres_xml_test.py b/end2end/flask_postgres_xml_test.py index 1fceb92fe..87d8794e1 100644 --- a/end2end/flask_postgres_xml_test.py +++ b/end2end/flask_postgres_xml_test.py @@ -6,6 +6,8 @@ # e2e tests for flask_postgres sample app post_url_fw = "http://localhost:8092/xml_post" post_url_nofw = "http://localhost:8093/xml_post" +text_url_fw = "http://localhost:8092/xml_post_text" +text_url_nofw = "http://localhost:8093/xml_post_text" def test_firewall_started_okay(): events = fetch_events_from_mock("http://localhost:5000") @@ -31,8 +33,8 @@ def test_dangerous_response_with_firewall(): time.sleep(5) # Wait for attack to be reported events = fetch_events_from_mock("http://localhost:5000") - attacks = filter_on_event_type(events, "detected_attack") - + attacks = [a for a in filter_on_event_type(events, "detected_attack") if a["request"]["route"] == "/xml_post"] + assert len(attacks) == 1 del attacks[0]["attack"]["stack"] @@ -54,3 +56,33 @@ def test_dangerous_response_without_firewall(): res = requests.post(post_url_nofw, data=xml_data) assert res.status_code == 200 +def test_dangerous_element_text_with_firewall(): + xml_data = 'Malicious dog\', TRUE); --' + res = requests.post(text_url_fw, data=xml_data) + assert res.status_code == 500 + + time.sleep(5) # Wait for attack to be reported + events = fetch_events_from_mock("http://localhost:5000") + attacks = [a for a in filter_on_event_type(events, "detected_attack") if a["request"]["route"] == "/xml_post_text"] + + assert len(attacks) == 1 + del attacks[0]["attack"]["stack"] + + assert attacks[0]["attack"] == { + "blocked": True, + "kind": "sql_injection", + 'metadata': { + 'dialect': "postgres", + 'sql': "INSERT INTO dogs (dog_name, isAdmin) VALUES ('Malicious dog', TRUE); --', FALSE)" + }, + 'operation': "psycopg2.Connection.Cursor.execute", + 'pathToPayload': ".dog.[0]", + 'payload': "\"Malicious dog', TRUE); --\"", + 'source': "xml", + 'user': None + } + +def test_dangerous_element_text_without_firewall(): + xml_data = 'Malicious dog\', TRUE); --' + res = requests.post(text_url_nofw, data=xml_data) + assert res.status_code == 200 diff --git a/sample-apps/flask-postgres-xml/app.py b/sample-apps/flask-postgres-xml/app.py index fa4497471..74ce559c5 100644 --- a/sample-apps/flask-postgres-xml/app.py +++ b/sample-apps/flask-postgres-xml/app.py @@ -68,6 +68,34 @@ def post_xml(): conn.close() return f'Dogs created successfully' +@app.route("/xml_post_text", methods=['POST']) +def post_xml_text(): + raw_xml = request.data.decode('utf-8') + root = ET.fromstring(raw_xml) + conn = get_db_connection() + cursor = conn.cursor() + for dog in root.findall('dog'): + dog_name = dog.text + cursor.execute(f"INSERT INTO dogs (dog_name, isAdmin) VALUES ('%s', FALSE)" % (dog_name)) + conn.commit() + cursor.close() + conn.close() + return f'Dogs created successfully' + +@app.route("/xml_post_text_lxml", methods=['POST']) +def post_xml_text_lxml(): + raw_xml = request.data.decode('utf-8') + root = ET2.fromstring(raw_xml) + conn = get_db_connection() + cursor = conn.cursor() + for dog in root.findall('dog'): + dog_name = dog.text + cursor.execute(f"INSERT INTO dogs (dog_name, isAdmin) VALUES ('%s', FALSE)" % (dog_name)) + conn.commit() + cursor.close() + conn.close() + return f'Dogs created successfully' + @app.route("/xml_post_lxml", methods=['POST']) def post_xml_lxml(): raw_xml = request.data.decode('utf-8')