Various crashes from fuzzing

[stusmall]

This is an issue I found while fuzzing this morning. Here is a simplified test case:

let context = Context::new();
let _ = Tera::one_off("{{1/0}}", &context, true);

[Keats]

Thanks, should be easy to fix!

[stusmall]

I'm finding more issues. Would you like an issue for each or should I start one parent issue for all my fuzzing finds?

[Keats]

You can probably convert this issue to be about all fuzzing errors and I will go through them

[stusmall]

Sounds good. I'll check upstream with the fuzzing trophy case on how best to track that since I already committed this issue on there. I'll do that when I get into the office.

For now I also have:

"{{W>W>vv}}{"

which generates:

thread 'test_fatal_template' panicked at 'internal error: entered unreachable code: unimplemented math expression for Logic(LogicExpr { lhs: Expr { val: Ident("W"), negated: false, filters: [] }, rhs: Expr { val: Ident("W"), negated: false, filters: [] }, operator: Gt })', src/renderer/processor.rs:688:18
note: Some details are omitted, run with RUST_BACKTRACE=full for a verbose backtrace.

[stusmall]

{{0%0}}

which generates

thread 'test_fatal_template' panicked at 'attempt to calculate the remainder with a divisor of zero', src/renderer/processor.rs:658:47
note: Some details are omitted, run with RUST_BACKTRACE=full for a verbose backtrace.
stack backtrace:

[stusmall]

{{2.00%0.000}}"

This one looks like another easy fix, changing:
Some(Number::from_f64(ll % rr).unwrap())
to
Number::from_f64(ll % rr)
cleaned it up

[stusmall]

{{22220222222022222220}}

causes

thread 'test_fatal_template' panicked at 'called Result::unwrap() on an Err value: ParseIntError { kind: Overflow }', src/libcore/result.rs:997:5
note: Some details are omitted, run with RUST_BACKTRACE=full for a verbose backtrace.
stack backtrace:

[stusmall]

I uploaded a patch. It is incomplete, as you can even see in the diff. There are some places where I would fix a problem just to leave another example of the same style of unwrap alone. I was just fixing each individual issue to get the fuzzer to move along the find the next one. I wanted to make the patch available to you, but don't expect it to be merged in due to how incomplete it is so I didn't even bother making a PR.

IIRC I fixed all the issues in this thread except {{22220222222022222220}}. Once I saw that code it looked like it would need more than just a simple one liner so I stepped away.

If that gets fixed up and you'd like me to continue fuzzing just @ me and I'd be happy to pop back in or I can provide the code I'm using and some instructions.

[Keats]

I pushed tests + fixes for all the one mentioned in the thread, can you share the code used to fuzz?

[stusmall]

I added the code here: stusmall@30e211b

To run it use this project: https://github.com/rust-fuzz/cargo-fuzz

and kick it off with cargo fuzz run fuzz_template

[Keats]

Thanks, it's running now!

[Keats]

Found a couple more issues thanks to cargo-fuzz! I'm running into some weird issue now AddressSanitizer: heap-buffer-overflow on address on the v1 branch which seems unrelated to Tera.

[stusmall]

Did you end up chasing those errors coming from outside tera? If not I'd like to take a look.

[Keats]

I didn't look further, please do!

[stusmall]

What was the input that caused the heap-buffer-overflow on address issue?

[Keats]

No clue sorry, it did look like a crash of the fuzzer itself rather than Tera. I changed laptop since then so can't re-run it right now.

[stusmall]

No problem. Thanks! Since these issues got fixed I'm going to close this out. If I find anything else I'll just reopen this issue.

[stusmall]

"{_{{W~1+11}}k{" causes a panic

[stusmall]

"{{n~n<n.11}}}"

[Keats]

Just to be sure, are you running it on the v1 branch?

[stusmall]

Yes. Commit hash 470badb

[Keats]

Both are fixed at the grammar level in the v1 branch. I love those issues, it really highlights some poor decisions in the grammar :p

[stusmall]

That's good to hear! I was afraid I was bringing a bunch of small, pedantic issues. I'll rebase and fire up the fuzzer again. Also I found the heap buffer overflow issue, thanks for calling that one out. I've got a bug with the project and will work with them on the fix. Hopefully should get something in soonish, but until them I've got a crummy work around so I can keep moving.

[Keats]

I was afraid I was bringing a bunch of small, pedantic issues

Not at all! They were all kind of huge issues in the parser.

[Keats]

I have seen your post on the v-htmlescape about the overflow since I ran into it as well.
I need to benchmark again to see whether a basic implementation is a big difference in terms of perf and whether it's worth having the dependency.

[stusmall]

Someone else in another thread called out that it might be a false positive from asan/rust interaction. I plan on getting to the root of that one this weekend. I wouldn't want to get them remove as a dependency prematurely over a false positive. I'll have a confirmation on it soon.

[stusmall]

I followed up on the v_htmlescape issue. It isn't a false positive.

[stusmall]

On commit 8c694a8 I found two more:

{{266314325266577770*4167}}7}}7

6/0}}}}}}}}}}}}

[Keats]

Thanks, I fixed the first one but the second one doesn't panic for me?

[stusmall]

Weird. Me either. Sorry about that.

[Keats]

Beta 5 released with the latest changes. How do you work around the v-htmlescape bug? Just commenting out the code?

[stusmall]

I'd found a switch that looked like it would turn off the SIMD optimizations. I thought that would turn it off but it's still hitting a buffer overflow on that avx instruction. Either the switch isn't respected or I misunderstood what it did.

That was a dead end, sorry about that.

[Keats]

No worries, I'm going to have a look at alternatives for the escaping. I've noticed that pulldown-cmark wrote their own version of escape_html/escape_href as well for example.

[Keats]

Found {{0~1~``~0~0~177777777777777777777~``~0~0~h}} after a few hours
Edit: fixed

[Keats]

It doesn't like I can trigger another panic, except from the v-htmlescape

[stusmall]

I'll go ahead and close this out then! Thank you so much. If a fuzzer finds a new issue we can reopen this issue.