Skip to content

Commit

Permalink
Remove: Drop usage of defusedxml
Browse files Browse the repository at this point in the history
We use the defusedxml.lxml package which is actually deprecated.
Therefore replace it and use more secure settings for lxml when creating
the lxml parser object instead.
  • Loading branch information
bjoernricks authored and greenbonebot committed Jun 14, 2024
1 parent 068594e commit bb57e35
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 31 deletions.
19 changes: 9 additions & 10 deletions gvm/xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
from io import IOBase
from typing import AnyStr, List, Optional, TextIO, Union

import defusedxml.lxml as secET
from defusedxml import DefusedXmlException
from lxml.etree import Element as create_element
from lxml.etree import Error as EtreeError
from lxml.etree import LxmlError, SubElement, XMLParser
Expand Down Expand Up @@ -36,7 +34,8 @@ class XmlError(GvmError):
def create_parser():
# huge_tree => disable security restrictions and support very deep trees and
# very long text content (for get_reports)
return XMLParser(encoding="utf-8", huge_tree=True)
# resolve_entities=False => disable entity resolution for security reasons
return XMLParser(encoding="utf-8", huge_tree=True, resolve_entities=False)


def parse_xml(xml: AnyStr) -> Element:
Expand Down Expand Up @@ -80,7 +79,7 @@ def set_attributes(self, attrs: dict[str, str]) -> "XmlCommandElement":

def append_xml_str(self, xml_text: str) -> None:
"""Append a xml element in string format."""
node = secET.fromstring(xml_text)
node = parse_xml(xml_text)
self._element.append(node)

def to_string(self) -> str:
Expand Down Expand Up @@ -155,7 +154,7 @@ def pretty_print(
)
)
elif isinstance(xml, str):
tree = secET.fromstring(xml)
tree = parse_xml(xml)
file.write(
xml_to_string(tree, pretty_print=True).decode(
sys.getdefaultencoding() + "\n"
Expand All @@ -170,17 +169,17 @@ def pretty_print(
def validate_xml_string(xml_string: str):
"""Checks if the passed string contains valid XML
Raises a GvmError if the XML is invalid. Otherwise the function just
Raises a XmlError if the XML is invalid. Otherwise the function just
returns.
Arguments:
xml_string: XML string to validate
Raises:
GvmError: The xml string did contain invalid XML
XmlError: The xml string did contain invalid XML
"""
try:
secET.fromstring(xml_string)
except (DefusedXmlException, LxmlError) as e:
raise GvmError("Invalid XML", e) from e
parse_xml(xml_string)
except LxmlError as e:
raise XmlError(f"Invalid XML {xml_string!r}. Error was {e}") from e
13 changes: 1 addition & 12 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ packages = [{ include = "gvm" }, { include = "tests", format = "sdist" }]
python = "^3.9"
paramiko = ">=2.7.1"
lxml = ">=4.5.0"
defusedxml = ">=0.6"
typing-extensions = ">=4.9.0"

[tool.poetry.group.dev.dependencies]
Expand Down
15 changes: 7 additions & 8 deletions tests/xml/test_pretty_print.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,16 @@
from io import StringIO
from unittest.mock import patch

import defusedxml.lxml as secET
from pontos.testing import temp_directory

from gvm.errors import InvalidArgumentType
from gvm.xml import pretty_print
from gvm.xml import parse_xml, pretty_print


class PrettyPrintTestCase(unittest.TestCase):
def test_pretty_print_to_file(self):
xml_str = '<test><this><with id="a">and text</with></this></test>'
elem = secET.fromstring(xml_str)
elem = parse_xml(xml_str)
expected_xml_string = (
"<test>\n"
" <this>\n"
Expand All @@ -38,7 +37,7 @@ def test_pretty_print_to_file(self):

def test_pretty_print_to_stringio(self):
xml_str = '<test><this><with id="a">and text</with></this></test>'
elem = secET.fromstring(xml_str)
elem = parse_xml(xml_str)
expected_xml_string = (
"<test>\n"
" <this>\n"
Expand All @@ -59,7 +58,7 @@ def test_pretty_print_to_stringio(self):
@patch("sys.stdout", new_callable=StringIO)
def test_pretty_print_to_stdout(self, mock_stdout):
xml_str = '<test><this><with id="a">and text</with></this></test>'
elem = secET.fromstring(xml_str)
elem = parse_xml(xml_str)
expected_xml_string = (
"<test>\n"
" <this>\n"
Expand Down Expand Up @@ -110,7 +109,7 @@ def test_pretty_print_with_string_to_stdout(self, mock_stdout):

def test_pretty_print_with_dict(self):
xml_str = '<test><this><with id="a">and text</with></this></test>'
elem = secET.fromstring(xml_str)
elem = parse_xml(xml_str)
expected_xml_string = (
"<test>\n"
" <this>\n"
Expand All @@ -134,7 +133,7 @@ def test_pretty_print_with_dict(self):
def test_pretty_print_with_dict_str(self):
xml_str = '<test><this><with id="a">and text</with></this></test>'
no_xml = "</test>"
elem = secET.fromstring(xml_str)
elem = parse_xml(xml_str)
expected_xml_string = (
"<test>\n"
" <this>\n"
Expand All @@ -160,7 +159,7 @@ def test_pretty_print_no_xml(self):

def test_pretty_print_type_error(self):
xml_str = '<test><this><with id="a">and text</with></this></test>'
elem = secET.fromstring(xml_str)
elem = parse_xml(xml_str)

with self.assertRaises(TypeError):
pretty_print(elem, file="string")
Expand Down

0 comments on commit bb57e35

Please sign in to comment.