Skip to content

Commit c8e13db

Browse files
authored
pySCG pillar 664 base 409 (#944)
* pySCG: adding doc for CWE-664_CWE-409 as part of #531 * fixed md lint errors * removing wrongly merged file * added dummy 209 to allow referencing and linking, updated template with suggested changes Co authored by Dean and Hubert --------- Signed-off-by: Helge Wehder <[email protected]>
1 parent c4e4236 commit c8e13db

File tree

8 files changed

+579
-106
lines changed

8 files changed

+579
-106
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# CWE-209: Generation of Error Message Containing Sensitive Information
2+
3+
Dummy file to be repaced during PR <https://github.com/ossf/wg-best-practices-os-developers/pull/945>

docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-409/README.md

Lines changed: 363 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 80 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,101 @@
11
# SPDX-FileCopyrightText: OpenSSF project contributors
22
# SPDX-License-Identifier: MIT
3-
""" Compliant Code Example """
3+
# SPDX-FileCopyrightText: OpenSSF project contributors
4+
# SPDX-License-Identifier: MIT
5+
"""Compliant Code Example"""
6+
47
import zipfile
58
from pathlib import Path
69

710
MAXSIZE = 100 * 1024 * 1024 # limit is in bytes
8-
MAXAMT = 5 # max amount of files, includes directories in the archive
11+
MAXAMT = 1000 # max amount of files, includes directories in the archive
912

1013

1114
class ZipExtractException(Exception):
1215
"""Custom Exception"""
1316

1417

15-
def path_validation(input_path, base_path, permit_subdirs=True):
16-
"""Ensure to have only allowed path names"""
17-
test_path = (Path(base_path) / input_path).resolve()
18-
if permit_subdirs:
19-
if not Path(base_path).resolve() in test_path.resolve().parents:
20-
raise ZipExtractException(f"Filename {test_path} not in {Path(base_path)} directory")
21-
else:
22-
if test_path.parent != Path(base_path).resolve():
23-
raise ZipExtractException(f"Filename {test_path} not in {Path(base_path)} directory")
18+
def path_validation(filepath: Path, base_path: Path):
19+
"""Ensure to have only allowed path names
20+
21+
Args:
22+
filepath (Path): path to archive
23+
base_path (Path): path to folder for extracting archives
24+
25+
Raises:
26+
ZipExtractException: if a directory traversal is detected
27+
"""
28+
input_path_resolved = (base_path / filepath).resolve()
29+
base_path_resolved = base_path.resolve()
30+
31+
if not str(input_path_resolved).startswith(str(base_path_resolved)):
32+
raise ZipExtractException(
33+
f"Filename {str(input_path_resolved)} not in {str(base_path)} directory"
34+
)
35+
2436

37+
def extract_files(filepath: str, base_path: str, exist_ok: bool = True):
38+
"""Extract archive below base_path
2539
26-
def extract_files(file, base_path):
27-
"""Unpack zip file into base_path"""
28-
with zipfile.ZipFile(file, mode="r") as archive:
29-
dirs = []
30-
# Validation:
40+
Args:
41+
filepath (str): path to archive
42+
base_path (str): path to folder for extracting archives
43+
exist_ok (bool, optional): Overwrite existing. Defaults to True.
44+
45+
Raises:
46+
ZipExtractException: If there are to many files
47+
ZipExtractException: If there are to big files
48+
ZipExtractException: If a directory traversal is detected
49+
"""
50+
# TODO: avoid CWE-209: Generation of Error Message Containing Sensitive Information
51+
52+
with zipfile.ZipFile(filepath, mode="r") as archive:
53+
# limit number of files:
3154
if len(archive.infolist()) > MAXAMT:
32-
raise ZipExtractException(f"Metadata check: too many files, limit is {MAXAMT}")
33-
for zm in archive.infolist():
34-
if zm.file_size > MAXSIZE:
35-
raise ZipExtractException(f"Metadata check: {zm.filename} is too big, limit is {MAXSIZE}")
36-
path_validation(zm.filename, base_path)
37-
with archive.open(zm.filename, mode='r') as mte:
38-
read_data = mte.read(MAXSIZE + 1)
39-
if len(read_data) > MAXSIZE:
40-
raise ZipExtractException(f"File {zm.filename} bigger than {MAXSIZE}")
55+
raise ZipExtractException(
56+
f"Metadata check: too many files, limit is {MAXAMT}"
57+
)
4158

42-
if not Path(base_path).resolve().exists():
43-
Path(base_path).resolve().mkdir(exist_ok=True)
59+
# validate by iterating over meta data:
60+
for item in archive.infolist():
61+
# limit file size using meta data:
62+
if item.file_size > MAXSIZE:
63+
raise ZipExtractException(
64+
f"Metadata check: {item.filename} is bigger than {MAXSIZE}"
65+
)
4466

45-
for zm in archive.infolist():
46-
# Extraction - create directories
47-
if zm.is_dir():
48-
dirs.append(Path(base_path).resolve().joinpath(zm.filename))
67+
path_validation(Path(item.filename), Path(base_path))
4968

50-
for directory in dirs:
51-
Path.mkdir(directory)
69+
# create target folder
70+
Path(base_path).mkdir(exist_ok=exist_ok)
71+
72+
# preparing for extraction, need to create directories first
73+
# as they may come in random order to the files
74+
for item in archive.infolist():
75+
if item.is_dir():
76+
xpath = Path(base_path).joinpath(item.filename).resolve()
77+
xpath.mkdir(exist_ok=exist_ok)
78+
79+
# start of extracting files:
80+
for item in archive.infolist():
81+
if item.is_dir():
82+
continue
83+
# we got a file
84+
with archive.open(item.filename, mode="r") as filehandle:
85+
read_data = filehandle.read(MAXSIZE + 1)
86+
if len(read_data) > MAXSIZE:
87+
# meta data was lying to us, actual size is bigger:
88+
raise ZipExtractException(
89+
f"Reality check, {item.filename} bigger than {MAXSIZE}"
90+
)
91+
xpath = Path(base_path).joinpath(filehandle.name).resolve()
92+
with open(xpath, mode="wb") as filehandle:
93+
filehandle.write(read_data)
94+
print(f"extracted successfully below {base_path}")
5295

53-
for zm in archive.infolist():
54-
with archive.open(zm.filename, mode='r') as mte:
55-
xpath = Path(base_path).joinpath(mte.name).resolve()
56-
print(f"Writing file {xpath}")
57-
# Skip if directory
58-
if xpath not in dirs: # check if file is a directory
59-
with open(xpath, mode="wb") as filehandle:
60-
filehandle.write(read_data)
6196

97+
#####################
98+
# Trying to exploit above code example
99+
#####################
62100

63-
extract_files("zip_attack_test.zip", "ziptemp")
101+
extract_files("zip_attack_test.zip", "ziptemp")
Lines changed: 68 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,72 @@
11
# SPDX-FileCopyrightText: OpenSSF project contributors
22
# SPDX-License-Identifier: MIT
33
"""Code to create a simple zip bomb in python for test purposes"""
4-
import zipfile
4+
55
import os
6-
import sys
7-
8-
ZIPBOMB = "zipbombfile.txt"
9-
ZIPTRAVERSAL = "zipslipfile.txt"
10-
ZIPFILENAME = "zip_attack_test.zip"
11-
12-
# preparing zip bomb file
13-
with open(ZIPBOMB, 'w', encoding="utf-8") as filehandle:
14-
for line in range(1023 * 128):
15-
sys.stdout.write(f"Preparing bombfile by writing lines of zero's to {ZIPBOMB}: {line}\r")
16-
sys.stdout.flush()
17-
filehandle.write("0" * 1023 + "\n")
18-
filehandle.close()
19-
filesize = os.path.getsize(ZIPBOMB) / float(1024 * 1024)
20-
print(f"\n{ZIPBOMB} : {filesize:.2f} MB")
21-
22-
# preparing zip slip file
23-
with open(ZIPTRAVERSAL, 'a', encoding="utf-8") as filehandle:
24-
filehandle.write("Testfile, filename shows if its good or evil")
25-
filehandle.close()
26-
traversal_files = ["zip_normal.txt"]
27-
traversal_files.append("../" * 39 + "tmp/zip_slip_posix.txt")
28-
traversal_files.append(r"..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\..\Temp\zip_slip_windows.txt")
29-
30-
with zipfile.ZipFile(ZIPFILENAME, mode="w", compression=zipfile.ZIP_DEFLATED) as zf:
31-
for clone in range(4):
32-
print(f"Adding {ZIPBOMB + str(clone)} as {ZIPFILENAME}")
33-
zf.write(ZIPBOMB, ZIPBOMB + str(clone))
34-
print("Adding multiple zip slip file's:")
35-
for item in traversal_files:
36-
print(f"Adding traversal attack file TRAVERSAL_FILE as {item}")
37-
zf.write(ZIPTRAVERSAL, item)
38-
39-
print(f"Removing temporary files: {ZIPBOMB}, {ZIPTRAVERSAL}")
40-
os.remove(ZIPBOMB)
41-
os.remove(ZIPTRAVERSAL)
42-
43-
filesize = os.path.getsize(ZIPFILENAME) / float(1024 * 1024)
44-
print(f"\n{ZIPFILENAME} : {filesize:.2f} MB")
6+
import zipfile
7+
8+
9+
def create_zip_slip_and_bomb(zip_filename: str):
10+
"""Create a zip file with zip slip and zip bomb content for testing
11+
12+
Args:
13+
zip_filename (str): name of zip file
14+
"""
15+
with zipfile.ZipFile(zip_filename, "w", compression=zipfile.ZIP_DEFLATED) as zf:
16+
# Adding a normal safe file at the start
17+
zip_normal = "safe_dir/zip_normal.txt"
18+
print(f"Adding a harmless normal file {zip_normal}")
19+
zf.writestr(zip_normal, b"Just a safe file\n")
20+
21+
print("Creating zip_slip example files")
22+
#####################################################
23+
# Zip Slip attempts:
24+
# - file with path traversal for unix
25+
slip_file_name = "../" * 10 + "tmp/zip_slip_posix.txt"
26+
slip_file_data = b"This test file tries to escape the extraction directory!\n"
27+
zf.writestr(slip_file_name, slip_file_data)
28+
29+
# - File with path traversal for Windows
30+
# Internally we have zip use slash, even for Windows
31+
slip_file_name = "../" * 10 + "tmp/zip_slip_windows0.txt"
32+
zf.writestr(slip_file_name, slip_file_data)
33+
34+
# - File with path traversal for Windows
35+
# Old extractors mishandle backslash
36+
slip_file_name = ".." * 10 + "Temp\\zip_slip_windows1.txt"
37+
zf.writestr(slip_file_name, slip_file_data)
38+
39+
# - Traversal attack with mixed slashes for Windows
40+
# Old extractors mishandling mixed slashes
41+
slip_file_name = "..\\/" * 10 + "Temp\\zip_slip_windows2.txt"
42+
zf.writestr(slip_file_name, slip_file_data)
43+
44+
# - Absolute path with drive letter for Windows
45+
slip_file_name = "C:/Temp/zip_slip_windows3.txt"
46+
zf.writestr(slip_file_name, slip_file_data)
47+
48+
##################################################
49+
# Zip Bomb Attempts:
50+
# - With 150MB files filled with zeros
51+
bomb_uncompressed_size = 150 * 1024 * 1024 # 150 MB
52+
large_data = b"\0" * bomb_uncompressed_size
53+
filename = "zipbombfile"
54+
55+
# - trying to fake the metadata size to 1KB for the first one
56+
print(f"trying to add fake 1KB metadata for {filename}0.txt")
57+
info = zipfile.ZipInfo(f"{filename}0.txt)")
58+
info.compress_type = zipfile.ZIP_DEFLATED
59+
info.file_size = 1024 # 1 KB (fake size)
60+
zf.writestr(f"{filename}0.txt", large_data)
61+
62+
# - Some more large files
63+
print("Writing more large zipbombfile's")
64+
for i in range(1, 4):
65+
zf.writestr(f"{filename}{i}.txt", large_data)
66+
67+
filesize = os.path.getsize(zip_filename) / float(1024 * 1024)
68+
print(f"created \n{zip_filename} : {filesize:.2f} MB")
69+
70+
71+
if __name__ == "__main__":
72+
create_zip_slip_and_bomb("zip_attack_test.zip")

docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-409/noncomplian01.py renamed to docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-409/noncompliant01.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# SPDX-FileCopyrightText: OpenSSF project contributors
22
# SPDX-License-Identifier: MIT
3-
""" Non-compliant Code Example """
3+
"""Non-compliant Code Example"""
4+
45
import zipfile
56

67
with zipfile.ZipFile("zip_attack_test.zip", mode="r") as archive:

docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-409/noncompliant02.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
# SPDX-FileCopyrightText: OpenSSF project contributors
22
# SPDX-License-Identifier: MIT
3-
""" Non-compliant Code Example """
3+
"""Non-compliant Code Example"""
4+
45
import zipfile
56

6-
MAXSIZE = 100 * 1024 * 1024 # limit is in bytes
7+
MAXSIZE = 100 * 1024 * 1024 # limit is in bytes
78

89
with zipfile.ZipFile("zip_attack_test.zip", mode="r") as archive:
910
for member in archive.infolist():

docs/Secure-Coding-Guide-for-Python/readme.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ It is __not production code__ and requires code-style or python best practices t
5252
|[CWE-197: Numeric Truncation Error](CWE-664/CWE-197/README.md)||
5353
|[CWE-197: Control rounding when converting to less precise numbers](CWE-664/CWE-197/01/README.md)||
5454
|[CWE-400: Uncontrolled Resource Consumption](CWE-664/CWE-400/README.md)||
55-
|[CWE-409: Improper Handling of Highly Compressed Data (Data Amplification)](CWE-664/CWE-409/.)||
55+
|[CWE-409: Improper Handling of Highly Compressed Data (Data Amplification)](CWE-664/CWE-409/README.md)||
5656
|[CWE-410: Insufficient Resource Pool](CWE-664/CWE-410/README.md)||
5757
|[CWE-426: Untrusted Search Path](CWE-664/CWE-426/README.md)|[CVE-2015-1326](https://www.cvedetails.com/cve/CVE-2015-1326),<br/>CVSSv3.0: __8.8__,<br/>EPSS: __00.20__ (23.11.2023)|
5858
|[CWE-459: Incomplete Cleanup](CWE-664/CWE-459/README.md)||

0 commit comments

Comments
 (0)