diff mbox series

PATCH] expand: Add ifsfree to expand to fix a logic error that causes a buffer over-read

Message ID CAOKKkHc=bt4p1d4hMzFbQeE14mZJJ7d1REpJ-D_BQD02re1Pdw@mail.gmail.com (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show
Series PATCH] expand: Add ifsfree to expand to fix a logic error that causes a buffer over-read | expand

Commit Message

Alex Gorinson June 20, 2022, 6:27 p.m. UTC
Due to a logic error in the ifsbreakup function in expand.c if a
heredoc and normal command is run one after the other by means of a
semi-colon, when the second command drops into ifsbreakup the command
will be evaluated with the ifslastp/ifsfirst struct that was set when
the here doc was evaluated. This results in a buffer over-read that
can leak the program's heap, stack, and arena addresses which can be
used to beat ASLR.

Steps to Reproduce:
First bug:
cmd args: ~/exampleDir/example> dash
$ M='AAAAAAAAAAAAAAAAA'    <note: 17 A's>
$ q00(){
$ <<000;echo
$ ${D?$M$M$M$M$M$M}        <note: 6 $M's>
$ 000
$ }
$ q00                      <note: After the q00 is typed in, the leak
should be echo'd out; this works with ash, busybox ash, and dash and
with all option args.>

Patch:
Adding the following to expand.c will fix both bugs in one go.
(Thank you to Harald van Dijk and Michael Greenberg for doing the
heavy lifting for this patch!)
==========================
diff mbox series

Patch

--- a/src/expand.c
+++ b/src/expand.c
@@ -859,6 +859,7 @@ 
if (discard)
return -1;

+ifsfree();
sh_error("Bad substitution");
}

@@ -1739,6 +1740,7 @@ 
} else
msg = umsg;
}
+ifsfree();
sh_error("%.*s: %s%s", end - var - 1, var, msg, tail);
 }
==========================