Summary:	ASTERISK-10345: safe_asterisk incorrectly invokes /bin/sh instead of /bin/bash
Reporter:	Charles Ulrich (culrich)	Labels:
Date Opened:	2007-09-20 14:14:22	Date Closed:	2007-09-21 13:06:26
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	The /contrib/script/safe_asterisk script invocation line refers to /bin/sh. /bin/sh should only be invoked for a script that is compatible with the original Bourne shell, which safe_asterisk is not. It contains shell language constructs that are valid only in the Bourne Again shell, which is commonly invoked as /bin/bash. On most Linux distributions, this is a not a problem since /bin/sh is usually a symbolic link to /bin/bash. On other systems like FreeBSD and Ubuntu, /bin/sh is an actual Bourne shell, not /bin/bash. This script fails on these systems and because shell scripts are notoriously bad at error reporting, the error message on failure is not helpful at determining the cause of the failure. The proper fix to this is to either change the shell invocation line to invoke /bin/bash or modify the script to conform to Bourne shell syntax. There may be other shell scripts in the Asterisk distribution with this issue, but safe_asterisk is the only one that I have noticed thus far.
Comments:	By: Digium Subversion (svnbot) 2007-09-20 15:42:29 Repository: asterisk Revision: 83316 ------------------------------------------------------------------------ r83316 | russell | 2007-09-20 15:42:28 -0500 (Thu, 20 Sep 2007) | 3 lines Change safe_asterisk to explicitly ask for /bin/bash, as it uses bashisms. (closes issue ASTERISK-10345, reported by culrich) ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-09-20 15:43:28 Repository: asterisk Revision: 83317 ------------------------------------------------------------------------ r83317 | russell | 2007-09-20 15:43:28 -0500 (Thu, 20 Sep 2007) | 11 lines Merged revisions 83316 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r83316 | russell | 2007-09-20 16:01:20 -0500 (Thu, 20 Sep 2007) | 3 lines Change safe_asterisk to explicitly ask for /bin/bash, as it uses bashisms. (closes issue ASTERISK-10345, reported by culrich) ........ ------------------------------------------------------------------------ By: Tilghman Lesher (tilghman) 2007-09-20 16:18:42 Shouldn't part of this fix be detecting the location of bash, since other platforms do not put it in /bin (FreeBSD, for example, puts bash in /usr/local/bin)?  Also, Ubuntu does not install bash by default, so this change is kind of fatal now. By: Digium Subversion (svnbot) 2007-09-21 07:00:48 Repository: asterisk Revision: 83399 ------------------------------------------------------------------------ r83399 | simon.perreault | 2007-09-21 07:00:43 -0500 (Fri, 21 Sep 2007) | 152 lines Merged revisions 83229,83231,83233-83234,83251,83278,83293-83298,83317,83349-83351,83381 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r83229 | file | 2007-09-20 12:10:57 -0400 (Thu, 20 Sep 2007) | 2 lines Fix memory leaks in pbx_dundi, cdr_pgsql, and the configuration file parser. ................ r83231 | file | 2007-09-20 12:19:45 -0400 (Thu, 20 Sep 2007) | 15 lines Merged revisions 83230 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r83230 | file | 2007-09-20 13:17:24 -0300 (Thu, 20 Sep 2007) | 7 lines Fix a minor spelling error. (closes issue ASTERISK-10343) Reported by: flefoll Patches:      chan_sip.c.trunk.83071.inita-patch uploaded by flefoll (license 244)      chan_sip.c.br14.83070.inita-patch uploaded by flefoll (license 244) ........ ................ r83233 | russell | 2007-09-20 12:27:07 -0400 (Thu, 20 Sep 2007) | 3 lines Don't start the event processing thread until after forking. (reported by Simon on the -dev list, thanks!) ................ r83234 | file | 2007-09-20 12:28:00 -0400 (Thu, 20 Sep 2007) | 15 lines Merged revisions 83232 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r83232 | file | 2007-09-20 13:25:30 -0300 (Thu, 20 Sep 2007) | 7 lines Make sure the minimum T1 timer value is obeyed in all cases. (closes issue ASTERISK-10342) Reported by: flefoll Patches:      chan_sip.c.trunk.83071.retrans-patch uploaded by flefoll (license 244)      chan_sip.c.br14.83070.retrans-patch uploaded by flefoll (license 244) ........ ................ r83251 | qwell | 2007-09-20 13:10:14 -0400 (Thu, 20 Sep 2007) | 16 lines Merged revisions 83246 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r83246 | qwell | 2007-09-20 12:09:14 -0500 (Thu, 20 Sep 2007) | 8 lines If # is pressed after dialing an extension in DISA, stop trying to collect more digits. (closes issue ASTERISK-10329) Reported by: atis Patches:      app_disa.c.branch.patch uploaded by atis (license 242)      app_disa.c.trunk.patch uploaded by atis (license 242) ........ ................ r83278 | qwell | 2007-09-20 15:05:16 -0400 (Thu, 20 Sep 2007) | 1 line Fix a trivial typo, to test our new commit bot ................ r83293 | russell | 2007-09-20 15:11:31 -0400 (Thu, 20 Sep 2007) | 1 line trivial formatting change ................ r83294 | russell | 2007-09-20 15:17:16 -0400 (Thu, 20 Sep 2007) | 1 line fix spelling in a comment ................ r83295 | russell | 2007-09-20 15:22:10 -0400 (Thu, 20 Sep 2007) | 1 line minor grammar fix ................ r83296 | russell | 2007-09-20 15:32:18 -0400 (Thu, 20 Sep 2007) | 1 line minor spelling fixes in a comment ................ r83297 | russell | 2007-09-20 15:42:33 -0400 (Thu, 20 Sep 2007) | 1 line trivial formatting change ................ r83298 | russell | 2007-09-20 15:45:00 -0400 (Thu, 20 Sep 2007) | 1 line trivial formatting change ................ r83317 | russell | 2007-09-20 17:02:17 -0400 (Thu, 20 Sep 2007) | 11 lines Merged revisions 83316 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r83316 | russell | 2007-09-20 16:01:20 -0500 (Thu, 20 Sep 2007) | 3 lines Change safe_asterisk to explicitly ask for /bin/bash, as it uses bashisms. (closes issue ASTERISK-10345, reported by culrich) ........ ................ r83349 | russell | 2007-09-20 17:17:39 -0400 (Thu, 20 Sep 2007) | 12 lines Merged revisions 83348 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r83348 | russell | 2007-09-20 16:16:48 -0500 (Thu, 20 Sep 2007) | 4 lines When daemonizing, don't change working directory to "/".  It makes it not be able to do a core dump when not running as uid=root. (closes issue ASTERISK-10340, xrg) ........ ................ r83350 | mmichelson | 2007-09-20 17:21:28 -0400 (Thu, 20 Sep 2007) | 3 lines Merging changes from queue_refcount_trunk into trunk. Refcounted queues now in place. ................ r83351 | mmichelson | 2007-09-20 17:37:28 -0400 (Thu, 20 Sep 2007) | 4 lines Oops. Getting rid of svnmerge-integrated and automerge stuff ................ r83381 | qwell | 2007-09-20 19:14:30 -0400 (Thu, 20 Sep 2007) | 8 lines More NEW_CLI conversions. (issue ASTERISK-10302) Patches:      app_playback.c.patch uploaded by moy (license 222)      app_minivm.c.patch uploaded by eliel (license 64)      astmm.c.patch uploaded by eliel (license 64) ................ ------------------------------------------------------------------------ By: Charles Ulrich (culrich) 2007-09-21 08:45:26 (I hope this isn't poor etiquette to click the "Reopen" button for this when the issue is fixed, but I'd like to respond to Corydon76's note 70861 and could not find any other way to do so. It appears that I cannot add a note to a bug once it is closed, even if I am the reporter. Corydon76 can do so by virtue of being an administrator.) It would be nice to detect the proper shell executable automatically. However, this would be far from a foolproof endeavour. The best option would be to rewrite the script using only POSIX-compliant syntax. In any event, this script didn't work on FreeBSD before. Now it at least works on Ubuntu. As far as I can tell, the assertion that bash is not installed on Ubuntu is incorrect. On all of the Ubuntu machines that I've set up, bash is installed and is the primary login shell for users. But it is not the target of the /bin/sh symlink because they've substituted a much faster shell named dash for startup scripts and other POSIX-compliant scripts. The Ubuntu developers posted a well-argued rationale for this, but I'm unable to locate it at the moment. By: Tilghman Lesher (tilghman) 2007-09-21 09:17:00 The Ubuntu developers' rationale came down to "we don't care if we break 10,000 packages, because we're right, and everybody else is wrong".  Not exactly a very user-friendly attitude, but it is what it is. By: Charles Ulrich (culrich) 2007-09-21 12:43:35 Scripts which use bash features shouldn't be invoking /bin/sh, no matter what the attitude of the Ubuntu developers. The issue is fixed and it was not my intent to begin an argument about it, so this will be my last note in this bug. By: Russell Bryant (russell) 2007-09-21 13:06:26 Agreed - it is certainly a bug on our part to invoke /bin/sh with a script that actually requires bash.  Thanks for pointing it out.