Skip to content

Commit

Permalink
Fix bug in md_parse_local_constant_sig().
Browse files Browse the repository at this point in the history 
Update style and add some defensive code checks.
  • Loading branch information
@AaronRobinsonMSFT
AaronRobinsonMSFT committed Nov 24, 2023
1 parent a955ead commit 9a7b1bb
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 29 deletions.
10 changes: 5 additions & 5 deletions src/dnmd/entry.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,16 +432,16 @@ static bool dump_table_rows(mdtable_t* table)
}
else if (sequence_points->records[k].kind == mdsp_HiddenSequencePointRecord)
{
printf("hidden-sequence-point-record: %u", sequence_points->records[k].hidden_sequence_point.il_offset);
printf("hidden-sequence-point-record: %u", sequence_points->records[k].hidden_sequence_point.rolling_il_offset);
}
else if (sequence_points->records[k].kind == mdsp_SequencePointRecord)
{
printf("sequence-point-record: (%u, %u, %" PRId64 ", %" PRId64 ", %" PRId64 ")",
sequence_points->records[k].sequence_point.il_offset,
sequence_points->records[k].sequence_point.num_lines,
sequence_points->records[k].sequence_point.rolling_il_offset,
sequence_points->records[k].sequence_point.delta_lines,
sequence_points->records[k].sequence_point.delta_columns,
sequence_points->records[k].sequence_point.start_line,
sequence_points->records[k].sequence_point.start_column);
sequence_points->records[k].sequence_point.rolling_start_line,
sequence_points->records[k].sequence_point.rolling_start_column);
}
else
{
Expand Down
29 changes: 19 additions & 10 deletions src/dnmd/pdb_blobs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ md_blob_parse_result_t md_parse_sequence_points(
if (delta_lines == 0 && delta_columns == 0)
{
sequence_points->records[i].kind = mdsp_HiddenSequencePointRecord;
sequence_points->records[i].hidden_sequence_point.il_offset = il_offset;
sequence_points->records[i].hidden_sequence_point.rolling_il_offset = il_offset;
continue;
}

Expand Down Expand Up @@ -276,11 +276,11 @@ md_blob_parse_result_t md_parse_sequence_points(
}

sequence_points->records[i].kind = mdsp_SequencePointRecord;
sequence_points->records[i].sequence_point.il_offset = il_offset;
sequence_points->records[i].sequence_point.num_lines = delta_lines;
sequence_points->records[i].sequence_point.rolling_il_offset = il_offset;
sequence_points->records[i].sequence_point.delta_lines = delta_lines;
sequence_points->records[i].sequence_point.delta_columns = delta_columns;
sequence_points->records[i].sequence_point.start_line = start_line;
sequence_points->records[i].sequence_point.start_column = start_column;
sequence_points->records[i].sequence_point.rolling_start_line = start_line;
sequence_points->records[i].sequence_point.rolling_start_column = start_column;
}

if (blob_len != 0)
Expand Down Expand Up @@ -314,7 +314,6 @@ md_blob_parse_result_t md_parse_local_constant_sig(mdhandle_t handle, uint8_t co
}

size_t required_size = sizeof(md_local_constant_sig_t) + num_custom_modifiers * sizeof(local_constant_sig->custom_modifiers[0]);

if (local_constant_sig == NULL || *buffer_len < required_size)
{
*buffer_len = required_size;
Expand All @@ -323,7 +322,7 @@ md_blob_parse_result_t md_parse_local_constant_sig(mdhandle_t handle, uint8_t co

local_constant_sig->custom_modifier_count = num_custom_modifiers;

for (uint8_t i = 0; i < num_custom_modifiers; ++i)
for (uint32_t i = 0; i < num_custom_modifiers; ++i)
{
uint32_t element_type;
if (!decompress_u32(&custom_modifiers_blob, &custom_modifiers_blob_len, &element_type))
Expand Down Expand Up @@ -395,13 +394,15 @@ md_blob_parse_result_t md_parse_local_constant_sig(mdhandle_t handle, uint8_t co
local_constant_sig->primitive.type_code = (uint8_t)type_code;
local_constant_sig->value_blob = blob;
local_constant_sig->value_len = blob_len;
break;
case ELEMENT_TYPE_R8:
if (blob_len != 8)
return mdbpr_InvalidBlob;
local_constant_sig->constant_kind = mdck_PrimitiveConstant;
local_constant_sig->primitive.type_code = (uint8_t)type_code;
local_constant_sig->value_blob = blob;
local_constant_sig->value_len = blob_len;
break;
case ELEMENT_TYPE_STRING:
local_constant_sig->constant_kind = mdck_PrimitiveConstant;
local_constant_sig->primitive.type_code = (uint8_t)type_code;
Expand All @@ -420,6 +421,7 @@ md_blob_parse_result_t md_parse_local_constant_sig(mdhandle_t handle, uint8_t co
case ELEMENT_TYPE_U4:
case ELEMENT_TYPE_I8:
case ELEMENT_TYPE_U8:
// Save off the value.
local_constant_sig->value_blob = blob;
local_constant_sig->value_len = blob_len;
switch (type_code)
Expand Down Expand Up @@ -458,8 +460,12 @@ md_blob_parse_result_t md_parse_local_constant_sig(mdhandle_t handle, uint8_t co
return mdbpr_InvalidBlob;
break;
}
default:
assert(false);
return mdbpr_InvalidArgument;
}

// Check if there is any remaining blob data.
if (blob_len == 0)
{
local_constant_sig->constant_kind = mdck_PrimitiveConstant;
Expand All @@ -468,18 +474,23 @@ md_blob_parse_result_t md_parse_local_constant_sig(mdhandle_t handle, uint8_t co
else
{
// If we have data remaining, then we need to read the enum type.
// In this case, the we need to subtract off the rest of the blob length from the value blob length
// In this case, we subtract off the rest of the blob length from the value blob length
// as it isn't part of the value.
local_constant_sig->value_len -= blob_len;
if (!decompress_u32(&blob, &blob_len, &constant_type_index)
|| !decompose_coded_index(constant_type_index, mdtc_idx_coded | InsertCodedIndex(mdci_TypeDefOrRef), &constant_type_table, &constant_type_row))
{
return mdbpr_InvalidBlob;
}

local_constant_sig->constant_kind = mdck_EnumConstant;
local_constant_sig->enum_constant.type_code = (uint8_t)type_code;
local_constant_sig->enum_constant.enum_type = CreateTokenType(constant_type_table) | constant_type_row;
}
break;
default:
assert(false);
return mdbpr_InvalidArgument;
}
return mdbpr_Success;
}
Expand Down Expand Up @@ -560,12 +571,10 @@ md_blob_parse_result_t md_parse_imports(mdhandle_t handle, uint8_t const* blob,
return mdbpr_InvalidArgument;

uint32_t num_imports = get_num_imports(blob, blob_len);

if (num_imports == UINT32_MAX)
return mdbpr_InvalidBlob;

size_t required_size = sizeof(md_imports_t) + num_imports * sizeof(imports->imports[0]);

if (imports == NULL || *buffer_len < required_size)
{
*buffer_len = required_size;
Expand Down
25 changes: 11 additions & 14 deletions src/inc/dnmd_pdb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ typedef struct md_sequence_points__
} document;
struct
{
uint32_t il_offset;
uint32_t num_lines;
uint32_t rolling_il_offset;
uint32_t delta_lines;
int64_t delta_columns;
int64_t start_line;
int64_t start_column;
int64_t rolling_start_line;
int64_t rolling_start_column;
} sequence_point;
struct
{
uint32_t il_offset;
uint32_t rolling_il_offset;
} hidden_sequence_point;
};
} records[];
Expand All @@ -72,15 +72,13 @@ typedef struct md_local_constant_sig__
{
struct
{
uint8_t type_code;
uint8_t type_code; // ELEMENT_TYPE_* - ECMA-335 II.23.1.16
} primitive;

struct
{
uint8_t type_code;
mdToken enum_type;
uint8_t type_code; // ELEMENT_TYPE_* - ECMA-335 II.23.1.16
mdToken enum_type; // See ECMA-335 II.14.3 for Enum restrictions.
} enum_constant;

struct
{
enum
Expand All @@ -89,19 +87,18 @@ typedef struct md_local_constant_sig__
mdgc_Class,
mdgc_Object
} kind;
mdToken type;
mdToken type; // TypeDefOrRefOrSpecEncoded - ECMA-335 II.23.2.8
} general;
};

uint8_t const* value_blob;
size_t value_len;

uint32_t custom_modifier_count;

struct
{
bool required;
mdToken type;
bool required; // Differentiate modreq vs modopt.
mdToken type; // Custom modifier - ECMA-335 II.23.2.7
} custom_modifiers[];
} md_local_constant_sig_t;
md_blob_parse_result_t md_parse_local_constant_sig(mdhandle_t handle, uint8_t const* blob, size_t blob_len, md_local_constant_sig_t* local_constant_sig, size_t* buffer_len);
Expand Down

0 comments on commit 9a7b1bb

Please sign in to comment.