On 05/12/2022 15:02, Herbert Xu wrote:
On Mon, Jun 20, 2022 at 02:27:10PM -0400, Alex Gorinson wrote: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.> [...]Thanks for the report! I think it's better to add the ifsfree() call to the exception handling path as other sh_error calls may trigger this too.
FWIW, the possibility of other sh_error calls being overlooked is why I ended up doing it differently myself: I ended up with exverror() calling ifsfree(). It is a smaller change than this; it works because there is currently no case where preserving IFS regions after an error is wanted anyway, and I cannot imagine a future change that would change that. But it is possible that my imagination is lacking there.
Q: If exception != EXERROR, longjmp() is called without first calling ifsfree(), leaving IFS regions in place for a higher level to deal with. If execution ultimately ends up at exitreset(), which calls ifsfree() anyway, that is fine. Is that guaranteed to always be the case?
Cheers, Harald van Dijk
