Fix out-of-bounds read in statement tail parser

[arimah]

Fixes #975

I would love to include a regression test with this PR, but as the problem only occurs when reading random noise past the end of a string, I have no idea how to accomplish such a feat.

[mceachen]

LGTM. This codebase doesn't have this tradition, but I've found it super handy to add a link to the issue (say, right before line 862) explaining the exotic/missing-incr for loop.

@Prinzhorn does this PR make your random-failure test harness pass?

[Prinzhorn]

@kkrypt0nn can you verify this locally? Does it fix the randomness for you? I can't look into this rn.

[kkrypt0nn]

Tested locally with similar code as in my comment in the issue. Result was consistent and working as expected.

Many thanks for your 🔥 investigation @arimah

[arimah]

Happy to help! 😄 Hope we can get this merged and released soon – just need code owner review too 👀

[mceachen]

@JoshuaWise can you take a look please?

[JoshuaWise]

Thanks for finding the source of this bug.

The only problem I see is that your solution breaks when there's a newline after -- or when there's a closed quote like /* foo */. My original code accounted for those cases by incrementing the pointer one less byte (knowing that the ++tail would be increment it again at the top of the loop), but I forgot to account for the cases where the inner loop terminates due to a nul byte. Your code accounts for the latter case, but not the --\n and /* foo */ cases.

Also, we should add tests for all of these cases 😅

[arimah]

Ahhhh, you're quite right, @JoshuaWise. These dang nested loops! We should be able to solve that by doing tail += 2; when a delimited comment ends, as well as adding an extra increment if we encounter \n inside a -- comment. I will update my code accordingly, and add a few tests for the cases you have just (rightly) pointed out.

[arimah]

O-kay, I think that does it now. The tail parser should (touch wood) skip the right number of characters in the right places now, and I've added a variety of little test cases to that effect. Everything passes locally, let's hope the pipeline here concurs.

@mceachen @JoshuaWise Please have a look and maybe we can get this little bug fixed 🙏

[JoshuaWise]

LGTM, thanks!