Refactor parser #23

keur · 2020-02-20T01:47:56Z

No description provided.

keur · 2020-02-20T01:49:21Z

The only parts I didn't audit heavily were ExtensionWhitelistParser::deserialize and ExtensionWhitelistParser::serialize. It uses sscanf (as mentioned in the issue), but that isn't unsafe because %x just reads a byte (the size of the list that is being serialized).

iefremov · 2020-02-20T05:57:55Z

@keur %x reads not a byte but a hex integer, i.e. unlimited amount of hex characters.

keur · 2020-02-20T16:08:56Z

Right right. Fixed. Please take another look @iefremov

iefremov · 2020-02-24T11:53:07Z

@keur do you mind replacing ST_EXTENSION_WHITELIST_DATA with std::string and HashSet with std::unordered_set as a first step of refactoring? It would much simpler than tossing around with raw C strings and fixing leaks here and there.

keur · 2020-04-05T09:03:49Z

@iefremov okay went ahead and changed those types

extension_set.h

extension_whitelist_parser.cc

extension_whitelist_parser.h

extension_whitelist_parser.cc


        Refactor code assuming buffer is 32 bytes

This is a safe assumption to make. https://support.google.com/chrome/a/answer/7532015?hl=en


        Simplify whitelist data serialization

Every extension ID that needs be serialized is 32 bytes. No longer necessary to embed the length of each extension in the serialized buffer.


        extension_whitelist_parser: safely serialize uint32_t

Include inttypes.h to format width-based integral types. This is safer than %x, because %x may read an unbounded amount of characters.


        Refactor with std functions

* Replace ST_EXTENSION_WHITELIST_DATA with std::string * Replace hash-set.cpp with std::unordered_set


        Review fixups

* Use strings where possible * Replace DECIMAL_STRING_MAX WITH UINT32_SERIALIZE_MAX since we are only serializing uint32_t * Remove extension_set class and make Serialize and Deserialize free functions

keur requested review from diracdeltas, bbondy and iefremov Feb 20, 2020