Bug 195918
| Summary: | /bin/sh crash caused by a particular script | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Yuri Victorovich <yuri> | ||||||||
| Component: | bin | Assignee: | Jilles Tjoelker <jilles> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Only Me | CC: | jilles, junovitch | ||||||||
| Priority: | --- | Flags: | jilles:
mfc-stable10+
jilles: mfc-stable9- jilles: mfc-stable8- |
||||||||
| Version: | 10.1-STABLE | ||||||||||
| Hardware: | Any | ||||||||||
| OS: | Any | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 150520 [details]
bin/sh with debug enable showing out of bounds array index
Created attachment 150521 [details]
bin/sh with debug enabled and check for negative value
Created attachment 150522 [details]
diff showing a check for negative value
An interesting observation to add, I can trigger this on my amd64 box but not on my i386 router. After further investigation, I found through using GDB on an old 9.1 VM with bin/sh compiled with debuging that expand.c runs atoi and uses the negative number it receives to read from an array index. I've attached the diff but it's crude and I don't think this is the "right" solution but does prevent any seg faults and errors out cleanly with the bad substitution. 64 bit: FreeBSD xts-bsd 10.1-RELEASE FreeBSD 10.1-RELEASE #0 r274401: Tue Nov 11 21:02:49 UTC 2014 root@releng1.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC amd64 jason@xts-bsd:/usr/src/bin/sh % sh $ echo b=${1985234857347568347:12:5} Segmentation fault 32 bit: FreeBSD xts-rtr 10.1-RELEASE FreeBSD 10.1-RELEASE #0 r274562M: Sun Nov 16 07:37:32 UTC 2014 root@xts-bsd:/usr/obj/nanobsd.soekris/i386.i386/usr/src/sys/GENERIC i386 jason@xts-rtr:~ % sh $ echo b=${1985234857347568347:12:5} ${1985234857347568347:1...}: Bad substitution I fixed this in 11-current a few months ago, in Subversion r268576, by using strtol() instead of atoi(). A commit references this bug: Author: jilles Date: Sun Dec 14 18:30:31 UTC 2014 New revision: 275777 URL: https://svnweb.freebsd.org/changeset/base/275777 Log: MFC r268576: sh: Correctly handle positional parameters beyond INT_MAX on 64-bit systems. Currently, there can be no more than INT_MAX positional parameters. Make sure to treat all higher ones as unset to avoid incorrect results and crashes. On 64-bit systems, our atoi() takes the low 32 bits of the strtol() and sign-extends them. On 32-bit systems, the call to atoi() returned INT_MAX for too high values and there is not enough address space for so many positional parameters, so there was no issue. PR: 195918 Changes: _U stable/10/ stable/10/bin/sh/expand.c stable/10/bin/sh/tests/parameters/Makefile stable/10/bin/sh/tests/parameters/positional5.0 |
I accidentally hit this crash trying some (probably bash) example taken from online. Crash is 100% reproducible. Script might be invalid for sh, but it shouldn't cause a crash. --- testcase --- $ sh $ echo b=${1985234857347568347:12:5} Segmentation fault (gdb) bt #0 0x0000000000409675 in argstr () #1 0x0000000000408af6 in expandarg () #2 0x0000000000405f41 in evalcommand () #3 0x000000000040534f in evaltree () #4 0x0000000000410529 in cmdloop () #5 0x0000000000410392 in main ()