Skip to content

Commit d56d0b9

Browse files
CopilotCopilot
authored
fix: harden input validation against malformed/malicious compressed data
fix: harden input validation against malformed/malicious compressed data
1 parent 901fb0d commit d56d0b9

5 files changed

Lines changed: 196 additions & 4 deletions

File tree

‎src/fzip.mbt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ pub fn decompress_sync(
1313
data : FixedArray[Byte],
1414
opts? : InflateOptions = InflateOptions::default(),
1515
) -> FixedArray[Byte] raise FzipError {
16-
if data.length() < 2 {
16+
if data.length() < 3 {
1717
return inflate_sync(data, opts~)
1818
}
1919
// Check for GZIP magic

‎src/gzip.mbt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ fn gzh(c : FixedArray[Byte], o : GzipOptions) -> Unit {
2929
///|
3030
/// Parse GZIP header and return data start offset
3131
fn gzs(d : FixedArray[Byte]) -> Int raise FzipError {
32+
if d.length() < 10 {
33+
raise fzip_err(InvalidHeader, msg="invalid gzip data")
34+
}
3235
if d[0] != b'\x1F' || d[1] != b'\x8B' || d[2] != b'\x08' {
3336
raise fzip_err(InvalidHeader, msg="invalid gzip data")
3437
}
@@ -40,6 +43,9 @@ fn gzs(d : FixedArray[Byte]) -> Int raise FzipError {
4043
// skip FNAME and FCOMMENT
4144
let mut zs = ((flg >> 3) & 1) + ((flg >> 4) & 1)
4245
while zs > 0 {
46+
if st >= d.length() {
47+
raise fzip_err(InvalidHeader, msg="invalid gzip data")
48+
}
4349
if d[st] == b'\x00' {
4450
zs -= 1
4551
}

‎src/security_wbtest.mbt‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
///|
2+
test "security/gzip - empty input" {
3+
let empty : FixedArray[Byte] = []
4+
let result = try? gunzip_sync(empty)
5+
assert_true(result is Err(_))
6+
}
7+
8+
///|
9+
test "security/gzip - too short input" {
10+
let short : FixedArray[Byte] = [b'\x1F', b'\x8B']
11+
let result = try? gunzip_sync(short)
12+
assert_true(result is Err(_))
13+
}
14+
15+
///|
16+
test "security/gzip - 9 bytes input" {
17+
let short : FixedArray[Byte] = [
18+
b'\x1F', b'\x8B', b'\x08', b'\x00', b'\x00', b'\x00', b'\x00', b'\x00',
19+
b'\x00',
20+
]
21+
let result = try? gunzip_sync(short)
22+
assert_true(result is Err(_))
23+
}
24+
25+
///|
26+
test "security/gzip - FNAME flag but truncated" {
27+
// GZIP header with FNAME flag (0x08) set but no actual filename data
28+
let truncated : FixedArray[Byte] = [
29+
b'\x1F', b'\x8B', b'\x08', b'\x08', b'\x00', b'\x00', b'\x00', b'\x00',
30+
b'\x00', b'\x03',
31+
]
32+
let result = try? gunzip_sync(truncated)
33+
assert_true(result is Err(_))
34+
}
35+
36+
///|
37+
test "security/zlib - empty input" {
38+
let empty : FixedArray[Byte] = []
39+
let result = try? unzlib_sync(empty)
40+
assert_true(result is Err(_))
41+
}
42+
43+
///|
44+
test "security/zlib - 1 byte input" {
45+
let short : FixedArray[Byte] = [b'\x78']
46+
let result = try? unzlib_sync(short)
47+
assert_true(result is Err(_))
48+
}
49+
50+
///|
51+
test "security/zlib - header only (no compressed data)" {
52+
// Valid zlib header (0x78 0x9C) but no compressed data or checksum
53+
let short : FixedArray[Byte] = [b'\x78', b'\x9C']
54+
let result = try? unzlib_sync(short)
55+
assert_true(result is Err(_))
56+
}
57+
58+
///|
59+
test "security/zip - empty input" {
60+
let empty : FixedArray[Byte] = []
61+
let result = try? unzip_sync(empty)
62+
assert_true(result is Err(_))
63+
}
64+
65+
///|
66+
test "security/zip - too short input" {
67+
let short : FixedArray[Byte] = [b'\x50', b'\x4B', b'\x05', b'\x06']
68+
let result = try? unzip_sync(short)
69+
assert_true(result is Err(_))
70+
}
71+
72+
///|
73+
test "security/zip - unzip_list empty input" {
74+
let empty : FixedArray[Byte] = []
75+
let result = try? unzip_list(empty)
76+
assert_true(result is Err(_))
77+
}
78+
79+
///|
80+
test "security/zip - path traversal detection" {
81+
// Test the is_unsafe_path function directly
82+
assert_true(is_unsafe_path("../etc/passwd"))
83+
assert_true(is_unsafe_path("foo/../../../etc/passwd"))
84+
assert_true(is_unsafe_path("..\\windows\\system32"))
85+
assert_true(is_unsafe_path("/etc/passwd"))
86+
assert_true(is_unsafe_path("\\windows\\system32"))
87+
assert_true(is_unsafe_path("C:\\windows\\system32"))
88+
assert_true(is_unsafe_path("c:/windows/system32"))
89+
assert_true(is_unsafe_path(".."))
90+
assert_true(is_unsafe_path("foo/.."))
91+
assert_true(is_unsafe_path("foo\\.."))
92+
// Safe paths
93+
assert_eq(is_unsafe_path("file.txt"), false)
94+
assert_eq(is_unsafe_path("dir/file.txt"), false)
95+
assert_eq(is_unsafe_path("dir/subdir/file.txt"), false)
96+
assert_eq(is_unsafe_path("..hidden"), false)
97+
assert_eq(is_unsafe_path("dir/..hidden/file.txt"), false)
98+
assert_eq(is_unsafe_path(""), false)
99+
}
100+
101+
///|
102+
test "security/decompress - 2 byte input no crash" {
103+
// decompress_sync with exactly 2 bytes should not crash
104+
// (previously accessed data[2] with only length >= 2 check)
105+
let data : FixedArray[Byte] = [b'\x00', b'\x00']
106+
let result = try? decompress_sync(data)
107+
// It's OK if this errors, just shouldn't crash with out-of-bounds
108+
let _ = result
109+
}
110+
111+
///|
112+
test "security/decompress - 1 byte input" {
113+
let data : FixedArray[Byte] = [b'\x00']
114+
let result = try? decompress_sync(data)
115+
let _ = result
116+
}
117+
118+
///|
119+
test "security/decompress - empty input" {
120+
let data : FixedArray[Byte] = []
121+
let result = try? decompress_sync(data)
122+
let _ = result
123+
}

‎src/zip.mbt‎

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,35 @@
1+
///|
2+
/// Check if a ZIP entry path is unsafe (path traversal or absolute path)
3+
fn is_unsafe_path(path : String) -> Bool {
4+
// Reject absolute paths
5+
if path.length() > 0 && (path[0] == '/' || path[0] == '\\') {
6+
return true
7+
}
8+
// Reject Windows absolute paths like "C:\..."
9+
if path.length() >= 3 &&
10+
((path[0] >= 'A' && path[0] <= 'Z') || (path[0] >= 'a' && path[0] <= 'z')) &&
11+
path[1] == ':' &&
12+
(path[2] == '/' || path[2] == '\\') {
13+
return true
14+
}
15+
// Check for path traversal components ("../", "..\")
16+
let l = path.length()
17+
let mut i = 0
18+
while i < l {
19+
// Check if we are at the start of a path component
20+
let at_start = i == 0 ||
21+
(i > 0 && (path[i - 1] == '/' || path[i - 1] == '\\'))
22+
if at_start && i + 1 < l && path[i] == '.' && path[i + 1] == '.' {
23+
// ".." at end of string, or followed by separator
24+
if i + 2 >= l || path[i + 2] == '/' || path[i + 2] == '\\' {
25+
return true
26+
}
27+
}
28+
i += 1
29+
}
30+
false
31+
}
32+
133
///|
234
/// Skip local ZIP file header, return offset past header+name+extra
335
fn slzh(d : FixedArray[Byte], b : Int) -> Int {
@@ -31,10 +63,23 @@ fn zh(
3163

3264
///|
3365
/// Read ZIP64 extra field
34-
fn z64e(d : FixedArray[Byte], b : Int) -> (Int, Int, Int) {
66+
fn z64e(
67+
d : FixedArray[Byte],
68+
b : Int,
69+
) -> (Int, Int, Int) raise FzipError {
3570
let mut b = b
36-
while b2(d, b) != 1 {
37-
b += 4 + b2(d, b + 2)
71+
while b + 4 <= d.length() && b2(d, b) != 1 {
72+
let field_len = b2(d, b + 2)
73+
if b + 4 + field_len > d.length() {
74+
raise fzip_err(InvalidZipData, msg="invalid zip64 extra field")
75+
}
76+
b += 4 + field_len
77+
}
78+
if b + 28 > d.length() {
79+
raise fzip_err(InvalidZipData, msg="invalid zip64 extra field")
80+
}
81+
if b2(d, b) != 1 {
82+
raise fzip_err(InvalidZipData, msg="invalid zip64 extra field")
3883
}
3984
(b8(d, b + 12).to_int(), b8(d, b + 4).to_int(), b8(d, b + 20).to_int())
4085
}
@@ -286,6 +331,9 @@ pub fn zip_sync(
286331
pub fn unzip_sync(
287332
data : FixedArray[Byte],
288333
) -> Array[(String, FixedArray[Byte])] raise FzipError {
334+
if data.length() < 22 {
335+
raise fzip_err(InvalidZipData, msg="data too short for zip archive")
336+
}
289337
let files : Array[(String, FixedArray[Byte])] = []
290338
// Find end of central directory
291339
let mut e = data.length() - 22
@@ -314,6 +362,12 @@ pub fn unzip_sync(
314362
}
315363
for _i in 0..<c {
316364
let (comp, sc, su, fn_str, no, off) = zh(data, o, z)
365+
if is_unsafe_path(fn_str) {
366+
raise fzip_err(
367+
InvalidZipData,
368+
msg="zip entry contains unsafe path: " + fn_str,
369+
)
370+
}
317371
let b_off = slzh(data, off)
318372
o = no
319373
if comp == 0 {
@@ -344,6 +398,9 @@ pub fn unzip_sync(
344398
pub fn unzip_list(
345399
data : FixedArray[Byte],
346400
) -> Array[UnzipFileInfo] raise FzipError {
401+
if data.length() < 22 {
402+
raise fzip_err(InvalidZipData, msg="data too short for zip archive")
403+
}
347404
let infos : Array[UnzipFileInfo] = []
348405
let mut e = data.length() - 22
349406
while e >= 0 && b4(data, e) != 0x06054B50U {

‎src/zlib.mbt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ fn zlh(c : FixedArray[Byte], o : ZlibOptions) -> Unit {
3434
///|
3535
/// Parse Zlib header and return data start offset
3636
fn zls(d : FixedArray[Byte], has_dict : Bool) -> Int raise FzipError {
37+
if d.length() < 2 {
38+
raise fzip_err(InvalidHeader, msg="invalid zlib data")
39+
}
3740
if (d[0].to_int() & 15) != 8 ||
3841
d[0].to_int() >> 4 > 7 ||
3942
((d[0].to_int() << 8) | d[1].to_int()) % 31 != 0 {
@@ -90,6 +93,9 @@ pub fn unzlib_sync(
9093
}
9194
let st = zls(data, has_dict)
9295
let end = data.length() - 4
96+
if end < st {
97+
raise fzip_err(InvalidHeader, msg="invalid zlib data")
98+
}
9399
let (buf, len) = inflt(
94100
data,
95101
InflateState::new(2),

0 commit comments

Comments
 (0)