HexBrain

shopify-scripts · Report #187305

Invalid handling of zero-length heredoc identifiers leads to infinite loop in the sandbox

Validstatus
highseverity
$10,000bounty
23upvotes
12 Jan 2017disclosed

How it was found

Introduction

Certain invalid Ruby programs (which should normally raise a syntax error) are able to cause an infinite loop in MRuby's parser which makes the mruby-engine sandbox (and consequently the MRI process it is running in) unresponsive to SIGTERM. The process begins looping forever and has to be terminated using SIGABRT or SIGKILL. The bug is caused by an improper handling of heredocs with a zero-length identifier.

Proof of concept

infinite_heredoc.rb:

<<''.a begin
  1. Save the above code as infinite_heredoc.rb.
  2. Run either: a) mruby infinite_heredoc.rb b) sandbox infinite_heredoc.rb
  3. Both cause an infinite loop, but b) also leaves the process unresponsive to SIGTERM.

Discussion

Everything below assumes the latest master of the mruby repository as of Dec 01th, which is commit 2cca9d368815e9c83a7489c40d69937d68cb43a2.

The <<''˙in the above POC code is parsed as a heredoc with an empty identifier. The rest of the POC is needed to bring the parser in a state where it is:

  1. Continually searching for the identifier.
  2. Erroneously thinking it found it, thereby signalling an end of the heredoc by pushing a tHEREDOC_END token.
  3. This token is then invalid for the current parser state, which makes it push an error token.
  4. Finally, while processing the error, the parser eventually calls parse_string again, which brings the process back to step 1, resulting in an infinite loop.

A variation of the bug, using do instead of begin:

infinite_heredoc_variation.rb:

<<''.a do

An excerpt from the parser's debug output, demonstrating the above:

Reading a token: Next token is token tHEREDOC_END ()
Error: discarding token tHEREDOC_END ()
Error: popping token error ()
Stack now 0 2 81 370 586 257 8 199
Shifting token error ()
Entering state 271
Reading a token: Next token is token tHEREDOC_END ()
Error: discarding token tHEREDOC_END ()
[...]

It is interesting to study what output MRI's parser gives for the same input:

infinite_heredoc.rb:1: can't find string "" anywhere before EOF
infinite_heredoc.rb:1: syntax error, unexpected end-of-input, expecting tSTRING_CONTENT or tSTRING_DBEG or tSTRING_DVAR or tSTRING_END
<<''.a begin
    ^

For a heredoc with a non-zero name, both MRuby and MRI produce similar outputs:

heredoc_valid_name.rb

<<'h'.a begin

MRuby output

heredoc_valid_name.rb:3:0: can't find heredoc delimiter "h" anywhere before EOF
heredoc_valid_name.rb:3:0: syntax error, unexpected $end

MRI output

heredoc_valid_name.rb:1: can't find string "h" anywhere before EOF
heredoc_valid_name.rb:1: syntax error, unexpected end-of-input, expecting tSTRING_CONTENT or tSTRING_DBEG or tSTRING_DVAR or tSTRING_END
<<'h'.a begin
    ^

Solution

The problematic code is located parse.y, function parse_string, starting at line 3956:

if ((len-1 == hinf->term_len) && (strncmp(s, hinf->term, len-1) == 0)) {
    return tHEREDOC_END;
}

The above code checks whether the current heredoc identifier can be matched and, if so, signals the end of the heredoc by returning a tHEREDOC_END token. The code is incorrect in the case when the length parameter is 0 due to the use of strncmp since it will return 0 even when the input strings are different (as is the case here, where s is "\n" and hinf->term is ""). Therefore, the check incorrectly succeeds when it shouldn't.

A possible fix is to check whether hinf->term_len != 0 in addition to the present checks so zero-length heredoc identifiers are invalidated.

empty_heredoc_identifier.patch

diff --git a/mrbgems/mruby-compiler/core/parse.y b/mrbgems/mruby-compiler/core/parse.y
index bf893fb..85150fc 100644
--- a/mrbgems/mruby-compiler/core/parse.y
+++ b/mrbgems/mruby-compiler/core/parse.y
@@ -3953,7 +3953,7 @@ parse_string(parser_state *p)
            --len;
        }
        }
-        if ((len-1 == hinf->term_len) && (strncmp(s, hinf->term, len-1) == 0)) {
+        if ((len-1 == hinf->term_len) && (strncmp(s, hinf->term, len-1) == 0) && (hinf->term_len != 0)) {
        return tHEREDOC_END;
        }
    }

With the provided patch, MRuby correctly terminates with the POC and issues an error message very similar to the one in MRI:

infinite_heredoc.rb:3:0: can't find heredoc delimiter "" anywhere before EOF
infinite_heredoc.rb:3:0: syntax error, unexpected $end

In addition, all the tests pass.

-- Denis Kasak Damir Jelić

This report was disclosed publicly on HackerOne and belongs to its author and the program. See the original