Skip to content

harden msf/pdb parsing against malformed radbin input #859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
momomuchu wants to merge 1 commit into
base: master
Choose a base branch
from
+250 −7
Open

harden msf/pdb parsing against malformed radbin input #859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/msf/msf_parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ msf_raw_stream_table_from_data(Arena *arena, String8 msf_data)

// (pages)
U32 page_size = ClampTop(page_size_raw, msf_data.size);


// a malformed/truncated file can drive page_size to 0 (ClampTop only
// caps the ceiling); reject it before it is ever used as a divisor below.
if (page_size != 0)
{

// (whole file page count)
U32 whole_file_page_count_max = CeilIntegerDiv(msf_data.size, page_size);
U32 whole_file_page_count = ClampTop(whole_file_page_count_raw, whole_file_page_count_max);
Expand Down Expand Up @@ -163,12 +168,15 @@ msf_raw_stream_table_from_data(Arena *arena, String8 msf_data)
B32 got_streams = 0;
MSF_RawStream *streams = 0;

if (got_directory) {
// the directory always begins with a 4-byte stream count; a malformed
// stream_table_size below 4 would make the stream_count read below go out
// of bounds and make (directory_size - 4) underflow into a ~1G count.
if (got_directory && directory_size >= 4) {
got_streams = 1;

// read stream count
U32 stream_count_raw = *(U32 *) directory_buf;

// setup counts, sizes, and offsets
U32 size_of_stream_entry = index_size == 2 ? 8 : 4;
U32 stream_count_max = (directory_size - 4) / size_of_stream_entry;
Expand Down Expand Up @@ -220,8 +228,9 @@ msf_raw_stream_table_from_data(Arena *arena, String8 msf_data)
result->stream_count = stream_count;
result->streams = streams;
}
} // page_size != 0
}

scratch_end(scratch);
return result;
}
Expand Down
15 changes: 13 additions & 2 deletions src/pdb/pdb_parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,11 @@ pdb_info_from_data(Arena *arena, String8 data)
result = push_array(arena, PDB_Info, 1);
result->first = first;
result->last = last;
result->auth_guid = *auth_guid;
// unrecognized PDB info versions (default case above) leave
// auth_guid unset; only dereference it when it was actually filled in.
if (auth_guid != 0) {
result->auth_guid = *auth_guid;
}
}

// read PDB features
Expand Down Expand Up @@ -457,8 +461,15 @@ pdb_gsi_from_data(Arena *arena, String8 data)
PDB_GsiParsed *result = push_array(arena, PDB_GsiParsed, 1);

// rjf: extract header
//
// `data` can be built by a caller from a malformed/out-of-range MSF stream
// (e.g. a stream number past the end of the stream table), which can arrive
// here as a null or otherwise invalid backing pointer paired with a
// wrapped-around (huge) size. Reject both a null pointer and a size outside
// the sane range for an MSF stream (all real stream sizes fit in a U32)
// before ever treating `data.str` as a header pointer.
PDB_GsiHeader *header = 0;
if(sizeof(*header) <= data.size)
if(data.str != 0 && data.size <= max_U32 && sizeof(*header) <= data.size)
{
header = (PDB_GsiHeader*)data.str;
}
Expand Down
223 changes: 223 additions & 0 deletions src/rdi_from_pdb/tests/rdi_from_pdb_tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,226 @@ Test(p2r_determinism)
TestCheck(mismatch_num == 0);
}
}

////////////////////////////////
//~ Regression coverage: malformed PDB/MSF input must not crash radbin
//
// Covers EpicGames/raddebugger #832, #833, #834, #835 -- each constructs a
// tiny (single-page-per-stream) but structurally valid MSF 7.0 container,
// then corrupts exactly the field the original report relied on. All inputs
// here are hand-built in-process (no external test data required).

internal String8
p2r_test_build_info_stream(Arena *arena, PDB_InfoVersion version, U32 hash_table_count)
{
U8 buf[128] = {0};
U8 *p = buf;
U32 v;
v = version; MemoryCopy(p, &v, 4); p += 4; // version
v = 0; MemoryCopy(p, &v, 4); p += 4; // time
v = 1; MemoryCopy(p, &v, 4); p += 4; // age
B32 recognized = (version == PDB_InfoVersion_VC70_DEP || version == PDB_InfoVersion_VC70 ||
version == PDB_InfoVersion_VC80 || version == PDB_InfoVersion_VC110 ||
version == PDB_InfoVersion_VC140);
if(recognized)
{
Guid guid = {0};
MemoryCopy(p, &guid, sizeof(guid)); p += sizeof(guid);
}
v = 0; MemoryCopy(p, &v, 4); p += 4; // names_len
v = hash_table_count; MemoryCopy(p, &v, 4); p += 4; // hash_table_count
v = hash_table_count; MemoryCopy(p, &v, 4); p += 4; // hash_table_max
v = 0; MemoryCopy(p, &v, 4); p += 4; // num_present_words
v = 0; MemoryCopy(p, &v, 4); p += 4; // num_deleted_words
if(hash_table_count > 0)
{
v = 0; MemoryCopy(p, &v, 4); p += 4; // relative_name_off
v = 0; MemoryCopy(p, &v, 4); p += 4; // stream number
}
U64 size = (U64)(p - buf);
U8 *result_buf = push_array_no_zero(arena, U8, size);
MemoryCopy(result_buf, buf, size);
return str8(result_buf, size);
}

internal String8
p2r_test_build_dbi_stream(Arena *arena, U16 gsi_sn, U16 psi_sn)
{
U8 buf[64] = {0};
U8 *p = buf;
U32 v32; U16 v16;
v32 = PDB_DbiHeaderSignature_V1; MemoryCopy(p, &v32, 4); p += 4; // sig
v32 = PDB_DbiVersion_70; MemoryCopy(p, &v32, 4); p += 4; // version
v32 = 1; MemoryCopy(p, &v32, 4); p += 4; // age
v16 = gsi_sn; MemoryCopy(p, &v16, 2); p += 2; // gsi_sn
v16 = 0; MemoryCopy(p, &v16, 2); p += 2; // build_number
v16 = psi_sn; MemoryCopy(p, &v16, 2); p += 2; // psi_sn
v16 = 0; MemoryCopy(p, &v16, 2); p += 2; // pdb_version
v16 = 0xFFFF; MemoryCopy(p, &v16, 2); p += 2; // sym_sn (unused by this test)
v16 = 0; MemoryCopy(p, &v16, 2); p += 2; // pdb_version2
v32 = 0; MemoryCopy(p, &v32, 4); p += 4; // module_info_size
v32 = 0; MemoryCopy(p, &v32, 4); p += 4; // sec_con_size
v32 = 0; MemoryCopy(p, &v32, 4); p += 4; // sec_map_size
v32 = 0; MemoryCopy(p, &v32, 4); p += 4; // file_info_size
v32 = 0; MemoryCopy(p, &v32, 4); p += 4; // tsm_size
v32 = 0; MemoryCopy(p, &v32, 4); p += 4; // mfc_index
v32 = 0; MemoryCopy(p, &v32, 4); p += 4; // dbg_header_size
v32 = 0; MemoryCopy(p, &v32, 4); p += 4; // ec_info_size
v16 = 0; MemoryCopy(p, &v16, 2); p += 2; // flags
v16 = 0; MemoryCopy(p, &v16, 2); p += 2; // machine
v32 = 0; MemoryCopy(p, &v32, 4); p += 4; // reserved
U64 size = (U64)(p - buf);
Assert(size == 64);
U8 *result_buf = push_array_no_zero(arena, U8, size);
MemoryCopy(result_buf, buf, size);
return str8(result_buf, size);
}

// Builds a minimal MSF 7.0 container with 6 fixed streams:
// 0: (empty) -- "old MSF directory" (unused, tolerated absent)
// 1: info_stream -- PDB Info Stream
// 2: (empty) -- Tpi (intentionally absent/invalid; tolerated)
// 3: dbi_stream -- DBI Stream
// 4: (empty) -- Ipi (intentionally absent/invalid; tolerated)
// 5: (empty) -- valid, in-range GSI/PSI target for the non-crashing cases
// `info_stream` and `dbi_stream` must each fit within a single 512-byte page.
internal String8
p2r_test_build_msf70(Arena *arena, String8 info_stream, String8 dbi_stream)
{
Assert(info_stream.size <= 512);
Assert(dbi_stream.size <= 512);

U32 page_size = 512;
U32 stream_count = 6;
U32 stream_sizes[6] = {0, (U32)info_stream.size, 0, (U32)dbi_stream.size, 0, 0};
U32 info_page = 3;
U32 dbi_page = 4;
U32 total_page_count = 5;
U32 directory_size = 4 + 4*stream_count + 4*2; // count + sizes[6] + 2 index entries (streams 1 & 3 each have 1 page)

U8 directory[512] = {0};
{
U8 *p = directory;
MemoryCopy(p, &stream_count, 4); p += 4;
for(U32 i = 0; i < stream_count; i += 1) { MemoryCopy(p, &stream_sizes[i], 4); p += 4; }
MemoryCopy(p, &info_page, 4); p += 4;
MemoryCopy(p, &dbi_page, 4); p += 4;
}

U8 header[512] = {0};
{
U8 *p = header;
MemoryCopy(p, msf_msf70_magic, sizeof(msf_msf70_magic)); p += sizeof(msf_msf70_magic);
U32 v;
v = page_size; MemoryCopy(p, &v, 4); p += 4; // page_size
v = 1; MemoryCopy(p, &v, 4); p += 4; // active_fpm
v = total_page_count; MemoryCopy(p, &v, 4); p += 4; // page_count
v = directory_size; MemoryCopy(p, &v, 4); p += 4; // stream_table_size
v = 0; MemoryCopy(p, &v, 4); p += 4; // reserved
v = 1; MemoryCopy(p, &v, 4); p += 4; // root_pn -> map page (page 1)
}

U8 map_page[512] = {0};
{ U32 v = 2; MemoryCopy(map_page, &v, 4); } // directory data lives on page 2

U8 *buf = push_array(arena, U8, total_page_count*page_size);
MemoryCopy(buf + 0*page_size, header, page_size);
MemoryCopy(buf + 1*page_size, map_page, page_size);
MemoryCopy(buf + 2*page_size, directory, page_size);
MemoryCopy(buf + info_page*page_size, info_stream.str, info_stream.size);
MemoryCopy(buf + dbi_page*page_size, dbi_stream.str, dbi_stream.size);

return str8(buf, (U64)total_page_count*page_size);
}

Test(p2r_malformed_input_hardening)
{
String8 radbin_path = test_build_exe_path(arena, s("radbin"));

String8 info_ok = p2r_test_build_info_stream(arena, PDB_InfoVersion_VC70, 0);
String8 dbi_ok = p2r_test_build_dbi_stream(arena, /*gsi_sn*/5, /*psi_sn*/5);

typedef struct { String8 name; String8 data; } MalformedCase;
MalformedCase cases[5];

// sanity: the "good" construction itself must round-trip without crashing
cases[0].name = s("good_baseline_sanity");
cases[0].data = p2r_test_build_msf70(arena, info_ok, dbi_ok);

// #832: MSF page_size field == 0 -> would divide by zero in msf_raw_stream_table_from_data
cases[1].name = s("832_msf_page_size_zero");
cases[1].data = str8_copy(arena, cases[0].data);
{
U32 zero = 0;
MemoryCopy(cases[1].data.str + sizeof(msf_msf70_magic), &zero, 4); // page_size is right after the magic
}

// #833: unrecognized PDB info version with hash_table_count>0 -> unset auth_guid dereferenced
cases[2].name = s("833_pdb_info_unknown_version_null_guid");
cases[2].data = p2r_test_build_msf70(arena, p2r_test_build_info_stream(arena, 0x12345678, 1), dbi_ok);

// #834 (and its duplicate #835): an out-of-range DBI stream number makes the
// PSI/GSI path build a String8 with a null base and a wrapped-around size,
// which pdb_gsi_from_data then treats as a header pointer. Here psi_sn is out
// of range; the size-range check in pdb_gsi_from_data is what makes this case
// fail without the fix (a plain null check alone would not), so this single
// case regression-locks that guard with teeth.
cases[3].name = s("834_dbi_psi_sn_out_of_range");
cases[3].data = p2r_test_build_msf70(arena, info_ok, p2r_test_build_dbi_stream(arena, /*gsi_sn*/5, /*psi_sn*/0xDEAD));

// MSF header stream_table_size < 4 -> directory_size < 4 -> out-of-bounds
// stream-count read and a (directory_size - 4) underflow into a ~1G count.
cases[4].name = s("msf_stream_table_size_too_small");
cases[4].data = str8_copy(arena, cases[0].data);
{
U32 zero = 0;
MemoryCopy(cases[4].data.str + OffsetOf(MSF_Header70, stream_table_size), &zero, 4);
}

String8 radbin_dir = str8_chop_last_slash(radbin_path);
for EachElement(idx, cases)
{
Temp scratch = scratch_begin(&arena, 1);
String8 pdb_name = str8f(scratch.arena, "%S.pdb", cases[idx].name);
String8 rdi_name = str8f(scratch.arena, "%S.rdi", cases[idx].name);
String8 out_name = str8f(scratch.arena, "%S.out", cases[idx].name);
t_write_file(pdb_name, cases[idx].data);
String8 pdb_path = t_make_file_path(scratch.arena, pdb_name);
String8 rdi_path = t_make_file_path(scratch.arena, rdi_name);
String8 out_path = t_make_file_path(scratch.arena, out_name);

// run radbin with stdout+stderr captured to out_path; delete any stale
// file first since AccessFlag_Append does not truncate on all platforms
delete_file_at_path(out_path);
File out_file = file_open(AccessFlag_Write|AccessFlag_Append|AccessFlag_ShareRead|AccessFlag_ShareWrite|AccessFlag_Inherited, out_path);
ProcessLaunchParams params = {0};
str8_list_push(scratch.arena, &params.cmd_line, radbin_path);
str8_list_push(scratch.arena, &params.cmd_line, pdb_path);
str8_list_push(scratch.arena, &params.cmd_line, str8f(scratch.arena, "--out:%S", rdi_path));
params.path = radbin_dir;
params.inherit_env = 1;
params.consoleless = 1;
params.stdout_file = out_file;
params.stderr_file = out_file;
Process process = process_launch(&params);
B32 launched = !process_match(process, process_zero());
B32 joined = launched ? process_join(process, max_U64, 0) : 0;
file_close(out_file);
String8 out_data = data_from_file_path(scratch.arena, out_path);

// radbin's exit code is not a reliable crash signal cross-platform (it can
// return a benign non-zero code, e.g. on Windows for these synthetic
// inputs), so detect a crash by the crash handler's output instead. Both
// handlers print "The process is terminating." on a fatal signal/exception
// (linux: "A fatal signal was received..."; windows: "A fatal exception...
// occurred..."); radbin never emits that string on a normal run.
String8 crash_sig = str8_lit("The process is terminating");
B32 crashed = str8_find_needle(out_data, 0, crash_sig, 0) < out_data.size;
if(crashed)
{
test_outf(" case \"%S\": radbin crash signature detected:\n%S\n", cases[idx].name, out_data);
}
TestCheck(launched && joined && !crashed);
scratch_end(scratch);
}
}
Loading