Skip to content

Commit d4d4799

Browse files
authored
Merge pull request #264 from BrianPugh/oob
Security Fix: Prevent OOB window buffer access when decompressing a malicious pattern match at end of window buffer.
2 parents 572833a + c3c6b5d commit d4d4799

File tree

10 files changed

+2078
-1628
lines changed

10 files changed

+2078
-1628
lines changed

ctests/test_decompressor.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,29 @@ void test_decompressor_byte_by_byte(void) {
6767

6868
TEST_ASSERT_EQUAL_STRING("foo foo foo", output_buffer_complete);
6969
}
70+
71+
void test_decompressor_malicious_oob(void) {
72+
/*****
73+
* Tests the decompressor if we feed in a pattern at position WINDOW_SIZE - 1.
74+
* The compressor should never generate this.
75+
*/
76+
const unsigned char compressed[] = {
77+
0b01011000, // header (window_bits=10, literal_bits=8)
78+
0b00111111, // pattern of length 2 at WINDOW_SIZE -1
79+
0b11110000, // 4 bits of zero-padding
80+
};
81+
82+
tamp_res res;
83+
TampDecompressor d;
84+
unsigned char window_buffer[1 << 10];
85+
86+
res = tamp_decompressor_init(&d, NULL, window_buffer);
87+
TEST_ASSERT_EQUAL(res, TAMP_OK);
88+
89+
unsigned char output_buffer[32];
90+
size_t output_written_size;
91+
92+
res = tamp_decompressor_decompress(&d, output_buffer, sizeof(output_buffer), &output_written_size, compressed,
93+
sizeof(compressed), NULL);
94+
TEST_ASSERT_EQUAL(res, TAMP_OOB);
95+
}

ctests/test_runner.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ int main(void) {
1616

1717
// Decompressor tests
1818
RUN_TEST(test_decompressor_byte_by_byte);
19+
RUN_TEST(test_decompressor_malicious_oob);
1920

2021
// Compressor tests
2122
RUN_TEST(test_compressor_init);

poetry.lock

Lines changed: 2003 additions & 1613 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tamp/_c_common.pyx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@ ERROR_LOOKUP = {
1212
ctamp.TAMP_ERROR: Exception,
1313
ctamp.TAMP_EXCESS_BITS: ExcessBitsError,
1414
ctamp.TAMP_INVALID_CONF: ValueError,
15+
ctamp.TAMP_OOB: ValueError,
1516
}

tamp/_c_src/tamp/common.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ enum {
5757
TAMP_ERROR = -1, // Generic error
5858
TAMP_EXCESS_BITS = -2, // Provided symbol has more bits than conf->literal
5959
TAMP_INVALID_CONF = -3, // Invalid configuration parameters.
60+
TAMP_OOB = -4, // Out-of-bounds access detected in compressed data.
61+
// Indicates malicious or corrupted input data attempting to
62+
// reference memory outside the decompressor window buffer.
6063
};
6164
typedef int8_t tamp_res;
6265

tamp/_c_src/tamp/decompressor.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,11 +192,23 @@ tamp_res tamp_decompressor_decompress_cb(TampDecompressor *decompressor, unsigne
192192
match_size += decompressor->min_pattern_size;
193193
window_offset = bit_buffer >> (32 - decompressor->conf_window);
194194

195+
// Security check: validate that the pattern reference (offset + size) does not
196+
// exceed window bounds. Malicious compressed data could craft out-of-bounds
197+
// references to read past the window buffer, potentially leaking memory.
198+
// Cast to uint32_t prevents signed integer overflow.
199+
const uint32_t window_size = (1u << decompressor->conf_window);
200+
if (TAMP_UNLIKELY((uint32_t)window_offset >= window_size ||
201+
(uint32_t)window_offset + (uint32_t)match_size > window_size)) {
202+
return TAMP_OOB;
203+
}
204+
195205
// Apply skip_bytes
196206
match_size_skip = match_size - decompressor->skip_bytes;
197207
window_offset_skip = window_offset + decompressor->skip_bytes;
198208

199-
// Check if we are output-buffer-limited, and if so to set skip_bytes
209+
// Check if we are output-buffer-limited, and if so to set skip_bytes.
210+
// Next tamp_decompressor_decompress_cb we will re-decode the same
211+
// token, and skip the first skip_bytes of it.
200212
// Otherwise, update the decompressor buffers
201213
size_t remaining = output_end - output;
202214
if (TAMP_UNLIKELY((uint8_t)match_size_skip > remaining)) {

tamp/ctamp.pxd

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ cdef extern from "tamp/common.h":
2121
TAMP_ERROR = -1, # Generic error
2222
TAMP_EXCESS_BITS = -2, # Provided symbol has more bits than conf->literal
2323
TAMP_INVALID_CONF = -3, # Invalid configuration parameters.
24+
TAMP_OOB = -4, # Out-of-bounds access detected in compressed data.
2425

2526
void initialize_dictionary(unsigned char *buffer, size_t size, uint32_t seed);
2627
int compute_min_pattern_size(uint8_t window, uint8_t literal);

tests/test_cli.py

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,36 @@
77
except ImportError:
88
micropython = None
99

10+
_app_kwargs = {}
11+
1012
try:
1113
from unittest.mock import patch
1214

1315
from tamp.cli.main import app
1416
except ImportError:
1517
pass
18+
else:
19+
from importlib.metadata import version
20+
21+
from packaging.version import Version
22+
23+
_cyclopts_version = Version(version("cyclopts"))
24+
_app_kwargs = {"result_action": "return_value"} if _cyclopts_version >= Version("4.0.0") else {}
1625

1726
compressed_foo_foo_foo = bytes(
1827
# fmt: off
1928
[
2029
0b010_11_00_0, # header (window_bits=10, literal_bits=8)
21-
0b1_0110011, # literal "f"
30+
0b1_0110011, # literal "f"
2231
0b0_0_0_00100, # the pre-init buffer contains "oo" at index 131
23-
# size=2 -> 0b0
24-
# 131 -> 0b0010000011
25-
0b00011_1_00, # literal " "
26-
0b100000_0_1, # There is now "foo " at index 0
27-
0b000_00000, # size=4 -> 0b1000
28-
0b00000_0_11, # Just "foo" at index 0; size=3 -> 0b11
29-
0b00000000, # index 0 -> 0b0000000000
30-
0b00_000000, # 6 bits of zero-padding
32+
# size=2 -> 0b0
33+
# 131 -> 0b0010000011
34+
0b00011_1_00, # literal " "
35+
0b100000_0_1, # There is now "foo " at index 0
36+
0b000_00000, # size=4 -> 0b1000
37+
0b00000_0_11, # Just "foo" at index 0; size=3 -> 0b11
38+
0b00000000, # index 0 -> 0b0000000000
39+
0b00_000000, # 6 bits of zero-padding
3140
]
3241
# fmt: on
3342
)
@@ -42,14 +51,14 @@ def test_compress_file_to_stdout(self):
4251
test_file.write_bytes(b"foo foo foo")
4352

4453
with patch("sys.stdout.buffer.write") as mock_stdout:
45-
app(["compress", str(test_file)])
54+
app(["compress", str(test_file)], **_app_kwargs)
4655
mock_stdout.assert_called_once_with(compressed_foo_foo_foo)
4756

4857
def test_compress_stdin_to_stdout(self):
4958
with patch("sys.stdout.buffer.write") as mock_stdout, patch(
5059
"sys.stdin.buffer.read", return_value="foo foo foo"
5160
):
52-
app("compress")
61+
app("compress", **_app_kwargs)
5362
mock_stdout.assert_called_once_with(compressed_foo_foo_foo)
5463

5564
def test_decompress_file_to_stdout(self):
@@ -58,14 +67,14 @@ def test_decompress_file_to_stdout(self):
5867
test_file = tmp_dir / "test_input.tamp"
5968
test_file.write_bytes(compressed_foo_foo_foo)
6069
with patch("sys.stdout.buffer.write") as mock_stdout:
61-
app(["decompress", str(test_file)])
70+
app(["decompress", str(test_file)], **_app_kwargs)
6271
mock_stdout.assert_called_once_with(b"foo foo foo")
6372

6473
def test_decompress_stdin_to_stdout(self):
6574
with patch("sys.stdout.buffer.write") as mock_stdout, patch(
6675
"sys.stdin.buffer.read", return_value=compressed_foo_foo_foo
6776
):
68-
app("decompress")
77+
app("decompress", **_app_kwargs)
6978
mock_stdout.assert_called_once_with(b"foo foo foo")
7079

7180
def test_decompress_stdin_to_file(self):
@@ -74,5 +83,5 @@ def test_decompress_stdin_to_file(self):
7483
test_file = tmp_dir / "test_output.txt"
7584

7685
with patch("sys.stdin.buffer.read", return_value=compressed_foo_foo_foo):
77-
app(["decompress", "-o", str(test_file)])
86+
app(["decompress", "-o", str(test_file)], **_app_kwargs)
7887
self.assertEqual(test_file.read_text(), "foo foo foo")

wasm/src/tamp.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ export class TampDecompressor {
153153
* Decompress a chunk of data
154154
* @param input - Compressed input data
155155
* @returns Promise resolving to decompressed data
156+
* @throws {RangeError} When compressed data contains out-of-bounds references
157+
* @throws {DecompressionError} When decompression fails
156158
*/
157159
decompress(input: Uint8Array): Promise<Uint8Array>;
158160

@@ -197,6 +199,8 @@ export function compress(data: Uint8Array, options?: TampCallbackOptions): Promi
197199
* @param data - Compressed data to decompress
198200
* @param options - Decompression options
199201
* @returns Promise resolving to decompressed data
202+
* @throws {RangeError} When compressed data contains out-of-bounds references or invalid configuration
203+
* @throws {DecompressionError} When decompression fails
200204
*/
201205
export function decompress(data: Uint8Array, options?: TampOptions): Promise<Uint8Array>;
202206

wasm/src/tamp.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import TampModule from './tamp-module.mjs';
99
const TAMP_ERROR = -1;
1010
const TAMP_EXCESS_BITS = -2;
1111
const TAMP_INVALID_CONF = -3;
12+
const TAMP_OOB = -4;
1213

1314
class TampError extends Error {
1415
constructor(code, message, details = {}) {
@@ -79,6 +80,8 @@ function throwOnError(result, operation) {
7980
throw new ExcessBitsError(`${operation}: Symbol has more bits than configured literal size`, details);
8081
case TAMP_INVALID_CONF:
8182
throw new RangeError(`${operation}: Invalid configuration parameters`);
83+
case TAMP_OOB:
84+
throw new RangeError(`${operation}: Out-of-bounds access detected in compressed data`);
8285
case TAMP_ERROR:
8386
default:
8487
if (operation.toLowerCase().includes('compress')) {

0 commit comments

Comments
 (0)