Skip to content

Commit

Permalink
Merge pull request from GHSA-3rw2-wfc8-wmj5
Browse files Browse the repository at this point in the history
* Adding checks for malicious SVGs

* Fixing merge conflicts

---------

Co-authored-by: Dave Quinlan <83430497+daveqnet@users.noreply.github.com>
  • Loading branch information
galvana and daveqnet authored Jul 6, 2023
1 parent 5aea738 commit 8beaace
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 2 deletions.
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ colorama>=0.4.3
cryptography==38.0.3
dask==2022.9.2
deepdiff==6.3.0
defusedxml==0.7.1
expandvars==0.9.0
fastapi[all]==0.89.1
fastapi-caching[redis]==0.3.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
replace_dataset_placeholders,
replace_version,
)
from fides.api.util.unsafe_file_util import verify_zip
from fides.api.util.unsafe_file_util import verify_svg, verify_zip
from fides.config import CONFIG


Expand Down Expand Up @@ -214,6 +214,7 @@ def save_template(cls, db: Session, zip_file: ZipFile) -> None:
)
elif info.filename.endswith(".svg"):
if not icon_contents:
verify_svg(file_contents)
icon_contents = str_to_b64_str(file_contents)
else:
raise ValidationError(
Expand Down
24 changes: 24 additions & 0 deletions src/fides/api/util/unsafe_file_util.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,34 @@
from typing import Optional
from zipfile import ZipFile

from defusedxml.ElementTree import fromstring

from fides.api.common_exceptions import ValidationError

MAX_FILE_SIZE = 16 * 1024 * 1024 # 16 MB
CHUNK_SIZE = 1024


def verify_svg(contents: str) -> None:
"""
Verifies the provided SVG content.
This function checks the given SVG content string for potential issues and throws an exception if any are found.
It first attempts to parse the SVG content using 'defusedxml.fromstring'. If the parsing is unsuccessful, this
will raise an exception, indicating that the SVG content may contain unsafe XML.
:param contents: The SVG content as a string.
:raises ValidationError: If the SVG content contains unsafe XML or 'use xlink'
"""
try:
fromstring(contents)
except Exception:
raise ValidationError("SVG file contains unsafe XML.")

if "use xlink" in contents:
raise ValidationError("SVG files with xlink references are not allowed.")


def verify_zip(zip_file: ZipFile, max_file_size: Optional[int] = None) -> None:
"""
Function to safely verify the contents of zipped files. It prevents potential
Expand Down
49 changes: 48 additions & 1 deletion tests/ops/util/test_unsafe_file_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,57 @@

import pytest

from fides.api.util.unsafe_file_util import verify_zip
from fides.api.common_exceptions import ValidationError
from fides.api.util.unsafe_file_util import verify_svg, verify_zip
from tests.ops.test_helpers.saas_test_utils import create_zip_file


class TestVerifySvg:
def test_verify_svg(self):
verify_svg(
"""<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 100 100">
<circle cx="50" cy="50" r="40"/>
</svg>
"""
)

def test_verify_svg_no_laughing_allowed(self):
"""Test "billion laughs attack" is prevented"""
with pytest.raises(ValidationError) as exc:
verify_svg(
"""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<!DOCTYPE svg [
<!ENTITY lol "lol">
<!ELEMENT lolz (#PCDATA)>
<!ENTITY lol1 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">
<!ENTITY lol2 "&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;">
<!ENTITY lol3 "&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;">
<!ENTITY lol4 "&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;">
<!ENTITY lol5 "&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;">
<!ENTITY lol6 "&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;">
<!ENTITY lol7 "&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;">
<!ENTITY lol8 "&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;">
<!ENTITY lol9 "&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;">
]>
<svg>
<lolz>&lol9;</lolz>
</svg>
"""
)
assert "SVG file contains unsafe XML." in str(exc.value)

def test_verify_svg_with_xlink(self):
with pytest.raises(ValidationError) as exc:
verify_svg(
"""<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 100 100">
<circle id="circle" cx="50" cy="50" r="40"/>
<use xlink:href="#circle"/>
</svg>
"""
)
assert "SVG files with xlink references are not allowed." in str(exc.value)


class TestVerifyZip:
@pytest.fixture
def zip_file(self) -> BytesIO:
Expand Down

0 comments on commit 8beaace

Please sign in to comment.