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

[arnold]

To answer my own question, without the change we get an internal
error and a core dump. With the change we get a clean error exit.

Adam, I will get your patch into the code base.

Thank you!!

Arnold

arnold@skeeve.com wrote:

> 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