Re: Gawk 4-Byte Out Of Bounds Read and Seg Fault

[arnold]

Thanks Andy.

To answer your question, apparently the difference is not the assignment,
but the return of a non-NULL pointer for that case.

The test case is a syntax error; gawk will exit anyway. For cases like
these, I don't care that much about bad reads or memory leaks, nor
do I think it really necessary to add test cases to the test suite.

Adam --- do you see a case where this fix is necessary when there is
not a syntax error?  In other words, am I missing something, and is there
a reason to use your change?

Thanks,

Arnold

"Andrew J. Schorr" <aschorr@telemetry-investments.com> wrote:

> Hi,
>
> You can see the test case here:
>
> https://github.com/AdamVanScyoc/gawk/commit/0a3c822e6b446c367c45e098fae0fdb1ba4dfa21
>
> But why is it useful to set ip->opcode to Op_field_assign when
> it already has that value?
>
> static INSTRUCTION *
> make_assignable(INSTRUCTION *ip)
> {
>       switch (ip->opcode) {
>       case Op_push:
>               ip->opcode = Op_push_lhs;
>               return ip;
>       case Op_field_spec:
>               ip->opcode = Op_field_spec_lhs;
>               return ip;
>       case Op_subscript:
>               ip->opcode = Op_subscript_lhs;
>               return ip;
>       case Op_field_assign:
>               // XXX why is this assignment useful?
>               ip->opcode = Op_field_assign;
>               return ip;
>       default:
>               break;  /* keeps gcc -Wall happy */
>       }
>       return NULL;
> }
>
> Regards,
> Andy
>
> On Tue, May 24, 2022 at 11:57:06AM -0600, arnold@skeeve.com wrote:
> > Hi.
> > 
> > Thanks for the report and the diff. I think it looks reasonable but
> > will explore some more. Please send the new test case; it wasn't in
> > your diff.
> > 
> > Arnold
> > 
> > Adam Van Scyoc <avanscy@g.clemson.edu> wrote:
> > 
> > > Hi, thanks for your work maintaining Gawk.
> > >
> > > After fuzzing with the google address sanitizers (and reproduced in
> > > valgrind) I discovered there's a 4-byte out-of-bounds read with a very
> > > simple input script that uses getline (see attachment).
> > >
> > > I wrote a patch that fixes the OOB read and still passes all tests
> > > (including a new test that I wrote called getlnfa.awk as in "getline field
> > > assign," which is the opcode type that was unhandled causing the bug).
> > >
> > > I have my patch attached as a diff to this email but also you can check it
> > > out on my github fork of gawk: https://github.com/AdamVanScyoc/gawk
> > >
> > > This bug was reproduced both with the google address sanitizer and 
> > > valgrind
> > > on MacOS 12.3.1 and in an Ubuntu 22.04 docker container. Repro'ed on Gawk
> > > versions 5.1.60 and 5.1.1
> > >
> > > Let me know if there's anything further you need. Also there may be more
> > > bugs to come as I continue fuzzing.
> > >
> > > Thanks!
> > > -Adam Van Scyoc