From bd71bdd74252585a1d567a480fa97e19884fa3fb Mon Sep 17 00:00:00 2001 From: "ivan.penkov@gmail.com" Date: Wed, 25 Sep 2013 18:25:13 +0000 Subject: Adding stricter validation checks to various symbol parser functions. More specifically, the validation of the following record types is improved: - FILE records - FUNC records - Line record - PUBLIC records Adding unittests. Review URL: https://breakpad.appspot.com/632003 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1217 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/processor/basic_source_line_resolver.cc | 269 ++++++++++++++++++++-------- 1 file changed, 190 insertions(+), 79 deletions(-) (limited to 'src/processor/basic_source_line_resolver.cc') diff --git a/src/processor/basic_source_line_resolver.cc b/src/processor/basic_source_line_resolver.cc index 799045ac..62aa4138 100644 --- a/src/processor/basic_source_line_resolver.cc +++ b/src/processor/basic_source_line_resolver.cc @@ -32,12 +32,13 @@ // See basic_source_line_resolver.h and basic_source_line_resolver_types.h // for documentation. - +#include #include #include #include #include +#include #include #include #include @@ -308,100 +309,64 @@ CFIFrameInfo *BasicSourceLineResolver::Module::FindCFIFrameInfo( } bool BasicSourceLineResolver::Module::ParseFile(char *file_line) { - // FILE - file_line += 5; // skip prefix - - vector tokens; - if (!Tokenize(file_line, kWhitespace, 2, &tokens)) { - return false; - } - - int index = atoi(tokens[0]); - if (index < 0) { - return false; - } - - char *filename = tokens[1]; - if (!filename) { - return false; + long index; + char *filename; + if (SymbolParseHelper::ParseFile(file_line, &index, &filename)) { + files_.insert(make_pair(index, string(filename))); + return true; } - - files_.insert(make_pair(index, string(filename))); - return true; + return false; } BasicSourceLineResolver::Function* BasicSourceLineResolver::Module::ParseFunction(char *function_line) { - // FUNC
- function_line += 5; // skip prefix - - vector tokens; - if (!Tokenize(function_line, kWhitespace, 4, &tokens)) { - return NULL; + uint64_t address; + uint64_t size; + long stack_param_size; + char *name; + if (SymbolParseHelper::ParseFunction(function_line, &address, &size, + &stack_param_size, &name)) { + return new Function(name, address, size, stack_param_size); } - - uint64_t address = strtoull(tokens[0], NULL, 16); - uint64_t size = strtoull(tokens[1], NULL, 16); - int stack_param_size = strtoull(tokens[2], NULL, 16); - char *name = tokens[3]; - - return new Function(name, address, size, stack_param_size); + return NULL; } BasicSourceLineResolver::Line* BasicSourceLineResolver::Module::ParseLine( char *line_line) { - //
- vector tokens; - if (!Tokenize(line_line, kWhitespace, 4, &tokens)) { - return NULL; + uint64_t address; + uint64_t size; + long line_number; + long source_file; + + if (SymbolParseHelper::ParseLine(line_line, &address, &size, &line_number, + &source_file)) { + return new Line(address, size, source_file, line_number); } - - uint64_t address = strtoull(tokens[0], NULL, 16); - uint64_t size = strtoull(tokens[1], NULL, 16); - int line_number = atoi(tokens[2]); - int source_file = atoi(tokens[3]); - - // Valid line numbers normally start from 1, however there are functions that - // are associated with a source file but not associated with any line number - // (block helper function) and for such functions the symbol file contains 0 - // for the line numbers. Hence, 0 shoud be treated as a valid line number. - // For more information on block helper functions, please, take a look at: - // http://clang.llvm.org/docs/Block-ABI-Apple.html - if (line_number < 0) { - return NULL; - } - - return new Line(address, size, source_file, line_number); + return NULL; } bool BasicSourceLineResolver::Module::ParsePublicSymbol(char *public_line) { - // PUBLIC
- - // Skip "PUBLIC " prefix. - public_line += 7; - - vector tokens; - if (!Tokenize(public_line, kWhitespace, 3, &tokens)) { - return false; - } - - uint64_t address = strtoull(tokens[0], NULL, 16); - int stack_param_size = strtoull(tokens[1], NULL, 16); - char *name = tokens[2]; + uint64_t address; + long stack_param_size; + char *name; + + if (SymbolParseHelper::ParsePublicSymbol(public_line, &address, + &stack_param_size, &name)) { + // A few public symbols show up with an address of 0. This has been seen + // in the dumped output of ntdll.pdb for symbols such as _CIlog, _CIpow, + // RtlDescribeChunkLZNT1, and RtlReserveChunkLZNT1. They would conflict + // with one another if they were allowed into the public_symbols_ map, + // but since the address is obviously invalid, gracefully accept them + // as input without putting them into the map. + if (address == 0) { + return true; + } - // A few public symbols show up with an address of 0. This has been seen - // in the dumped output of ntdll.pdb for symbols such as _CIlog, _CIpow, - // RtlDescribeChunkLZNT1, and RtlReserveChunkLZNT1. They would conflict - // with one another if they were allowed into the public_symbols_ map, - // but since the address is obviously invalid, gracefully accept them - // as input without putting them into the map. - if (address == 0) { - return true; + linked_ptr symbol(new PublicSymbol(name, address, + stack_param_size)); + return public_symbols_.Store(address, symbol); } - - linked_ptr symbol(new PublicSymbol(name, address, - stack_param_size)); - return public_symbols_.Store(address, symbol); + return false; } bool BasicSourceLineResolver::Module::ParseStackInfo(char *stack_info_line) { @@ -495,4 +460,150 @@ bool BasicSourceLineResolver::Module::ParseCFIFrameInfo( return true; } +// static +bool SymbolParseHelper::ParseFile(char *file_line, long *index, + char **filename) { + // FILE + assert(strncmp(file_line, "FILE ", 5) == 0); + file_line += 5; // skip prefix + + vector tokens; + if (!Tokenize(file_line, kWhitespace, 2, &tokens)) { + return false; + } + + char *after_number; + *index = strtol(tokens[0], &after_number, 10); + if (!IsValidAfterNumber(after_number) || *index < 0 || + *index == std::numeric_limits::max()) { + return false; + } + + *filename = tokens[1]; + if (!filename) { + return false; + } + + return true; +} + +// static +bool SymbolParseHelper::ParseFunction(char *function_line, uint64_t *address, + uint64_t *size, long *stack_param_size, + char **name) { + // FUNC
+ assert(strncmp(function_line, "FUNC ", 5) == 0); + function_line += 5; // skip prefix + + vector tokens; + if (!Tokenize(function_line, kWhitespace, 4, &tokens)) { + return false; + } + + char *after_number; + *address = strtoull(tokens[0], &after_number, 16); + if (!IsValidAfterNumber(after_number) || + *address == std::numeric_limits::max()) { + return false; + } + *size = strtoull(tokens[1], &after_number, 16); + if (!IsValidAfterNumber(after_number) || + *size == std::numeric_limits::max()) { + return false; + } + *stack_param_size = strtol(tokens[2], &after_number, 16); + if (!IsValidAfterNumber(after_number) || + *stack_param_size == std::numeric_limits::max() || + *stack_param_size < 0) { + return false; + } + *name = tokens[3]; + + return true; +} + +// static +bool SymbolParseHelper::ParseLine(char *line_line, uint64_t *address, + uint64_t *size, long *line_number, + long *source_file) { + //
+ vector tokens; + if (!Tokenize(line_line, kWhitespace, 4, &tokens)) { + return false; + } + + char *after_number; + *address = strtoull(tokens[0], &after_number, 16); + if (!IsValidAfterNumber(after_number) || + *address == std::numeric_limits::max()) { + return false; + } + *size = strtoull(tokens[1], &after_number, 16); + if (!IsValidAfterNumber(after_number) || + *size == std::numeric_limits::max()) { + return false; + } + *line_number = strtol(tokens[2], &after_number, 10); + if (!IsValidAfterNumber(after_number) || + *line_number == std::numeric_limits::max()) { + return false; + } + *source_file = strtol(tokens[3], &after_number, 10); + if (!IsValidAfterNumber(after_number) || *source_file < 0 || + *source_file == std::numeric_limits::max()) { + return false; + } + + // Valid line numbers normally start from 1, however there are functions that + // are associated with a source file but not associated with any line number + // (block helper function) and for such functions the symbol file contains 0 + // for the line numbers. Hence, 0 should be treated as a valid line number. + // For more information on block helper functions, please, take a look at: + // http://clang.llvm.org/docs/Block-ABI-Apple.html + if (*line_number < 0) { + return false; + } + + return true; +} + +// static +bool SymbolParseHelper::ParsePublicSymbol(char *public_line, + uint64_t *address, + long *stack_param_size, + char **name) { + // PUBLIC
+ assert(strncmp(public_line, "PUBLIC ", 7) == 0); + public_line += 7; // skip prefix + + vector tokens; + if (!Tokenize(public_line, kWhitespace, 3, &tokens)) { + return false; + } + + char *after_number; + *address = strtoull(tokens[0], &after_number, 16); + if (!IsValidAfterNumber(after_number) || + *address == std::numeric_limits::max()) { + return false; + } + *stack_param_size = strtol(tokens[1], &after_number, 16); + if (!IsValidAfterNumber(after_number) || + *stack_param_size == std::numeric_limits::max() || + *stack_param_size < 0) { + return false; + } + *name = tokens[2]; + + return true; +} + +// static +bool SymbolParseHelper::IsValidAfterNumber(char *after_number) { + if (after_number != NULL && strchr(kWhitespace, *after_number) != NULL) { + return true; + } + return false; +} + } // namespace google_breakpad -- cgit v1.2.1