Skip to content

Commit 6e1023c

Browse files
committed
gh-152718: Reject oversized table counts in the profiling binary reader
1 parent ecdef17 commit 6e1023c

4 files changed

Lines changed: 111 additions & 0 deletions

File tree

Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,9 @@ class TestBinaryFormatValidation(BinaryFormatTestBase):
980980
HDR_OFF_STR_TABLE = 36
981981
HDR_OFF_FRAME_TABLE = 44
982982
FILE_HEADER_PLACEHOLDER_SIZE = 64
983+
FILE_FOOTER_SIZE = 32
984+
FTR_OFF_STRINGS = 0
985+
FTR_OFF_FRAMES = 4
983986

984987
def test_replay_rejects_more_threads_than_declared(self):
985988
"""Replay rejects files with more unique threads than the header declares."""
@@ -1041,6 +1044,85 @@ def test_replay_rejects_trailing_partial_sample_header(self):
10411044
reader.replay_samples(RawCollector())
10421045
self.assertEqual(str(cm.exception), "Truncated sample data: 1 trailing bytes")
10431046

1047+
# Minimum on-disk size of one table entry (see binary_io.h).
1048+
MIN_STRING_ENTRY_SIZE = 1
1049+
MIN_FRAME_ENTRY_SIZE = 7
1050+
1051+
def _read_offset(self, filename, hdr_off):
1052+
with open(filename, "rb") as raw:
1053+
raw.seek(hdr_off)
1054+
return struct.unpack("=Q", raw.read(8))[0]
1055+
1056+
def _patch_footer_count(self, filename, ftr_off, value):
1057+
size = os.path.getsize(filename)
1058+
with open(filename, "r+b") as raw:
1059+
raw.seek(size - self.FILE_FOOTER_SIZE + ftr_off)
1060+
raw.write(struct.pack("=I", value))
1061+
1062+
def test_open_rejects_string_count_larger_than_file(self):
1063+
"""Open rejects a footer string count larger than the file."""
1064+
samples = [[make_interpreter(0, [
1065+
make_thread(1, [make_frame("s.py", 10, "s")])
1066+
])]]
1067+
filename = self.create_binary_file(samples, compression="none")
1068+
size = os.path.getsize(filename)
1069+
str_off = self._read_offset(filename, self.HDR_OFF_STR_TABLE)
1070+
max_strings = (size - str_off) // self.MIN_STRING_ENTRY_SIZE
1071+
self._patch_footer_count(filename, self.FTR_OFF_STRINGS, 0xFFFFFFFF)
1072+
1073+
with self.assertRaises(ValueError) as cm:
1074+
with BinaryReader(filename):
1075+
pass
1076+
self.assertEqual(
1077+
str(cm.exception),
1078+
f"Invalid string count 4294967295 exceeds maximum "
1079+
f"possible {max_strings}",
1080+
)
1081+
1082+
def test_open_rejects_frame_count_larger_than_file(self):
1083+
"""Open rejects a footer frame count larger than the file."""
1084+
samples = [[make_interpreter(0, [
1085+
make_thread(1, [make_frame("f.py", 10, "f")])
1086+
])]]
1087+
filename = self.create_binary_file(samples, compression="none")
1088+
size = os.path.getsize(filename)
1089+
frame_off = self._read_offset(filename, self.HDR_OFF_FRAME_TABLE)
1090+
max_frames = (size - frame_off) // self.MIN_FRAME_ENTRY_SIZE
1091+
self._patch_footer_count(filename, self.FTR_OFF_FRAMES, 0xFFFFFFFF)
1092+
1093+
with self.assertRaises(ValueError) as cm:
1094+
with BinaryReader(filename):
1095+
pass
1096+
self.assertEqual(
1097+
str(cm.exception),
1098+
f"Invalid frame count 4294967295 exceeds maximum "
1099+
f"possible {max_frames}",
1100+
)
1101+
1102+
def test_open_accepts_frame_count_at_capacity_boundary(self):
1103+
"""A frame count at the file-size cap opens; one more is rejected."""
1104+
samples = [[make_interpreter(0, [
1105+
make_thread(1, [make_frame("f.py", 10, "f")])
1106+
])]]
1107+
filename = self.create_binary_file(samples, compression="none")
1108+
size = os.path.getsize(filename)
1109+
frame_off = self._read_offset(filename, self.HDR_OFF_FRAME_TABLE)
1110+
max_frames = (size - frame_off) // self.MIN_FRAME_ENTRY_SIZE
1111+
1112+
self._patch_footer_count(filename, self.FTR_OFF_FRAMES, max_frames)
1113+
with BinaryReader(filename):
1114+
pass
1115+
1116+
self._patch_footer_count(filename, self.FTR_OFF_FRAMES, max_frames + 1)
1117+
with self.assertRaises(ValueError) as cm:
1118+
with BinaryReader(filename):
1119+
pass
1120+
self.assertEqual(
1121+
str(cm.exception),
1122+
f"Invalid frame count {max_frames + 1} exceeds maximum "
1123+
f"possible {max_frames}",
1124+
)
1125+
10441126

10451127
class TestBinaryEncodings(BinaryFormatTestBase):
10461128
"""Tests specifically targeting different stack encodings."""
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix unbounded memory allocation in the :mod:`profiling.sampling` binary profile
2+
reader, which trusted the string and frame table counts declared in a file's
3+
footer before allocating. A malformed file can no longer make the reader
4+
attempt a huge allocation.

Modules/_remote_debugging/binary_io.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ static_assert(SAMPLE_HEADER_FIXED_SIZE == 13,
9191
static_assert(FILE_FOOTER_SIZE == 32,
9292
"FILE_FOOTER_SIZE must remain 32");
9393

94+
/* Minimum on-disk bytes per table entry: a string is a >=1-byte length
95+
* varint; a frame is six >=1-byte varints plus the opcode byte. */
96+
#define MIN_STRING_ENTRY_SIZE 1
97+
#define MIN_FRAME_ENTRY_SIZE 7
98+
9499
/* Buffer sizes: 512KB balances syscall amortization against memory use,
95100
* and aligns well with filesystem block sizes and zstd dictionary windows */
96101
#define WRITE_BUFFER_SIZE (512 * 1024)

Modules/_remote_debugging/binary_io_reader.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,16 @@ reader_decompress_samples(BinaryReader *reader, const uint8_t *data)
240240
static inline int
241241
reader_parse_string_table(BinaryReader *reader, const uint8_t *data, size_t file_size)
242242
{
243+
/* Reject a count larger than the remaining bytes can hold. */
244+
size_t max_strings =
245+
(file_size - reader->string_table_offset) / MIN_STRING_ENTRY_SIZE;
246+
if (reader->strings_count > max_strings) {
247+
PyErr_Format(PyExc_ValueError,
248+
"Invalid string count %u exceeds maximum possible %zu",
249+
reader->strings_count, max_strings);
250+
return -1;
251+
}
252+
243253
reader->strings = PyMem_Calloc(reader->strings_count, sizeof(PyObject *));
244254
if (!reader->strings && reader->strings_count > 0) {
245255
PyErr_NoMemory();
@@ -280,6 +290,16 @@ reader_parse_frame_table(BinaryReader *reader, const uint8_t *data, size_t file_
280290
}
281291
#endif
282292

293+
/* Reject a count larger than the remaining bytes can hold. */
294+
size_t max_frames =
295+
(file_size - reader->frame_table_offset) / MIN_FRAME_ENTRY_SIZE;
296+
if (reader->frames_count > max_frames) {
297+
PyErr_Format(PyExc_ValueError,
298+
"Invalid frame count %u exceeds maximum possible %zu",
299+
reader->frames_count, max_frames);
300+
return -1;
301+
}
302+
283303
size_t alloc_size = (size_t)reader->frames_count * sizeof(FrameEntry);
284304
reader->frames = PyMem_Malloc(alloc_size);
285305
if (!reader->frames && reader->frames_count > 0) {

0 commit comments

Comments
 (0)