Comment by hendzen

8 years ago

I think this bug is kind of an indictment of Ragel. It has some great ideas, but since the generated code is so low level - and allows arbitrary blocks of code to be executed in the guts of the parser, bugs like these can result in this horrible memory issues - particularly since the generated code is often used to parse untrusted user input.

14 comments

hendzen

Reply
jgrahamc  8 years ago

I don't blame Ragel.

  • eridius  8 years ago

    Ragel shares part of the blame. Why did it use a strict equality check when it could have trivially done a >=?

    • cperciva  8 years ago

      Even a >= check would have been suboptimal. Rather than

          /* generated code */
    if ( ++p == pe )
        goto _test_eof;

      or

          /* generated code */
    if ( ++p >= pe )
        goto _test_eof;

      they should have had

          /* generated code */
    if ( ++p == pe )
        goto _test_eof;
    assert(p < pe);

      since having servers core dumping would have drawn attention to the bug in a way that counting one byte too many and then hitting _test_eof would not.

      3 replies →

    • arestor  8 years ago

      It's C. If you have an array, you may only compare to one element behind the last. Everything else is undefined behavior. So a compiler may just "optimize" your >= to ==.

      4 replies →

    • pepve  8 years ago

      That's a great defensive technique. But even when you do that, the underlying bug should still be fixed. I don't think the equality operator is the underlying bug.

      Consecutive pointer increments without a bounds check in between sounds like a bug to me. But I don't really know Ragel, and perhaps the compiler doesn't have enough information to determine this is what's happening.

      1 reply →

    • andrewf  8 years ago

      Speculation: people may want to use Ragel-generated code in C++, where strict equality checks are idiomatic. p may be an iterator instead of a raw pointer. >= can be absent, or slower than ==.