The JSON-parsing vulnerability fixed by Steemd 0.20.9

in #steem6 years ago (edited)

if custom JSON was also vulnerable which inspired me to take a closer look.In my previous bug report on steemd, @crokkon asked

<p dir="auto">The JSON parsing code in the FC library used by Steem did have a check for nesting, but it was not adequate: <pre><code> /** the purpose of this check is to verify that we will not get a stack overflow in the recursive descent parser */ void check_string_depth( const string& utf8_str ) { int32_t open_object = 0; int32_t open_array = 0; for( auto c : utf8_str ) { switch( c ) { case '{': open_object++; break; case '}': open_object--; break; case '[': open_array++; break; case ']': open_array--; break; default: break; } FC_ASSERT( open_object < 100 && open_array < 100, "object graph too deep", ("object depth",open_object)("array depth", open_array) ); } } <p dir="auto">This count checks the depth of opening braces, but it has both false positives and false negatives, because it neglects to check whether the brace appears within a string. <p dir="auto">For example, the following JSON string would be viewed as having a large array depth, even though it does not have any arrays at all: <pre><code>{ "foo" : "[[[[[[...[[[" } <p dir="auto">(Example had to be truncated, see addendum below.) <p dir="auto">The other direction is more concerning; we can construct a malfomed JSON-RPC call which exhausts the stack of the caller, causing the process to crash. The following pattern can be repeated up the maximum size of an accepted message (2MB). <pre><code>{"jsonrpc":"2.0","method": ["]",["]",["]",["]",["]",["]",["]",["]",... <p dir="auto">This bug was reported to security@steem.com on January 2nd and fixed in Steemd version 0.20.9 on January 22nd. <h2>Analysis <p dir="auto">Stack-exhaustion attacks are not as easy to find with a coverage-based tool like AFL, as memory-exhaustion attacks are. It might be that setting a very low recursion depth or stack size might have allowed AFL to find this problem, but it seems likely that the necessary path through the code is one that's not easy for a purely coverage-based tool to find. I'd be interested in learning what other sort of tools would be better at identifying this sort of weakness. <p dir="auto"><span>Bitshares fixed this vulnerability far earlier, in March 2018: <a href="https://github.com/bitshares/bitshares-fc/commit/f9802f686007e7efeaa88cba31f647ca96f1ee0c" target="_blank" rel="nofollow noreferrer noopener" title="This link will take you away from hive.blog" class="external_link">https://github.com/bitshares/bitshares-fc/commit/f9802f686007e7efeaa88cba31f647ca96f1ee0c But, the fractured nature of the FC and Graphene ecosystem means that the vulnerability was not communicated to other implementations (or ignored if it was.) <h2>Addendum <p dir="auto">An earlier version of this article was rejected with the "object too deep" error, because the example above of a false positive triggered the code check I quoted! So while the false negative had been fixed, the false positive bug remains. <p dir="auto"><img src="https://images.hive.blog/768x0/https://cdn.steemitimages.com/DQmP8hhtEA1jciL8BW5RGNYPSpwYKtDyYxZdk2cEahTJGPS/etienne-steenkamp-1166506-unsplash.jpg" alt="etienne-steenkamp-1166506-unsplash.jpg" srcset="https://images.hive.blog/768x0/https://cdn.steemitimages.com/DQmP8hhtEA1jciL8BW5RGNYPSpwYKtDyYxZdk2cEahTJGPS/etienne-steenkamp-1166506-unsplash.jpg 1x, https://images.hive.blog/1536x0/https://cdn.steemitimages.com/DQmP8hhtEA1jciL8BW5RGNYPSpwYKtDyYxZdk2cEahTJGPS/etienne-steenkamp-1166506-unsplash.jpg 2x" /><br /> Photo by Etienne Steenkamp on Unsplash
Sort:  

Congratulations @fuzz-ai! You received a personal award!

Happy Birthday! - You are on the Steem blockchain for 1 year!

You can view your badges on your Steem Board and compare to others on the Steem Ranking

Vote for @Steemitboard as a witness to get one more award and increased upvotes!