Comment by thurston
8 years ago
Author of Ragel here.
An experienced Ragel programmer would know that when you start setting the EOF pointer you are enabling code paths that never executed before. Like, potentially buggy ones. Eek!
8 years ago
Author of Ragel here.
An experienced Ragel programmer would know that when you start setting the EOF pointer you are enabling code paths that never executed before. Like, potentially buggy ones. Eek!
I'm not sure if you've had a chance to look at the Cloudflare blog post yet (https://blog.cloudflare.com/incident-report-on-memory-leak-c...), but while they take full responsibility they do point out (under root cause of the bug) that a generated equality check could be >= instead which would avoid the bug. Obviously GIGO applies and it's their bug, but it might be worth seeing if there's anything you can do on Ragel side?
Well doing that would mean ragel would incorrectly read one character, rather than run off forever. Personally I'd rather have the latter. Much easier to catch with memory checkers. Eventually you try to read some thing you're not allowed to read, or blow something else up, instead of just read the first byte of the int following the buffer, or whatever.
There would have to be an additional bounds check when issuing a goto in an error action, but doing that is contrary to the simple execution model that ragel users have come to rely on.
Gotta ask the question, where was the testing when they altered 7 year old code without the involvement of the original developer?
I'm not familiar with Ragel, but you might consider adding support for SaferCPlusPlus[1] as an output target. It should be a fairly simple modification of your existing C or C++ output generator. SaferCPlusPlus was, in part, designed for this (I mean, CloudFlare's) sort of situation - using C/C++ in internet facing applications.
[1] https://github.com/duneroadrunner/SaferCPlusPlus
Actually, the Ragel generated code (fragment) looks like it's already pretty SaferCPlusPlus compliant, since it itself doesn't declare any buffers or pointer/iterators. So I guess the recommendation instead would be for CloudFlare, and anyone else writing nginx modules in C/C++, to wrap any nginx-supplied buffers in a bounds-checked wrapper (like an array_view[1], gsl::span, or RandomAccessSection[2]).
[1] https://github.com/rhysd/array_view
[2] https://github.com/duneroadrunner/SaferCPlusPlus#txscoperand...