$ dash -n /etc/init.d/shellinaboxd /etc/init.d/shellinaboxd: 15: Syntax error: "(" unexpected (expecting "}") > OpenRC are expected to be POSIX-compliant and must therefore avoid e.g. Bash-specific tests and constructs. https://devmanual.gentoo.org/tasks-reference/init-scripts/index.html Reproducible: Always Steps to Reproduce: 1. eselect sh set dash 2. rc-service shellinaboxd start
Created attachment 764868 [details] shellinaboxd.init-r1 This is my rewrite of the runscript. It is fully POSIX conforming, with the exception that it continues to use the local keyword. I left local in because, despite its behaviour being undefined by POSIX, it is both useful and widely implemented. In the worst case, a user might symlink /bin/sh to a strictly conforming implementation, in which case an error would appear with an extremely low probability of impacting the overall behaviour of the script. Various quality issues have been addressed. The definition of the label now takes into account all characters that could impact upon the grammar of the --user-css option. The command_args variable is no longer subject to an unquoted expansion but is instead handled by the criminally under-utilised xargs(1) utility. Thus, not only is it protected from unintentional word splitting and pathname expansion, but double quotes will be parsed helpfully; now even CSS pathnames that contain spaces will be handled without issue. Pathnames that contain double quotes still won't be, but that was already the case. Besides, xargs(1) can be expected to generate an error should that ever happen. I'm prepared to generate a patch directly against the gentoo repo but it's going to require a revision bump and I don't see any point in doing that until the fact that it doesn't build successfully is addressed. I don't think there's an open bug for that, so I'll file one shortly.
Created attachment 764923 [details] shellinaboxd.init-r1 (with changes by Anna) (In reply to Kerin Millar from comment #1) > This is my rewrite of the runscript. It is fully POSIX conforming, with the > exception that it continues to use the local keyword. I left local in > because, despite its behaviour being undefined by POSIX, it is both useful > and widely implemented. In the worst case, a user might symlink /bin/sh to a > strictly conforming implementation, in which case an error would appear with > an extremely low probability of impacting the overall behaviour of the > script. Thank you very much! But I suppose "Signed-off-by" thing is needed to use this in Gentoo. > The command_args variable is no longer subject to an > unquoted expansion but is instead handled by the criminally under-utilised > xargs(1) utility. Thus, not only is it protected from unintentional word > splitting and pathname expansion, but double quotes will be parsed > helpfully; now even CSS pathnames that contain spaces will be handled > without issue. Pathnames that contain double quotes still won't be, but that > was already the case. Besides, xargs(1) can be expected to generate an error > should that ever happen. I removed start()/stop() functions and used declarative config instead.
(In reply to jan Anja from comment #2) > Thank you very much! > But I suppose "Signed-off-by" thing is needed to use this in Gentoo. Indeed. I'll get to it. I'm still trying to figure out how to fix an OpenSSL-related build error. > I removed start()/stop() functions and used declarative config instead. I would have done the same thing, if it were not for the variable handling in OpenRC being of a fundamentally broken design. It appears to work as expected in most cases but this is only because its inputs tend to be trivial. In case you don't believe it, try going to the CSS directory and running the following. # cp black-on-white.css '$(echo gotcha).css' Now start shellinaboxd using your declarative version of the runscript. The following error will occur. [config] Cannot access style sheet "/usr/share/shellinabox-resources/gotcha.css"! Yes, that was a code injection - running as root. Imagine if the name were to contain a more destructive payload. Stuffing argument lists into scalar variables already makes the problem domain difficult to contend with (sh doesn't grant us the luxury of using arrays). Still, the singularly low calibre of the start-stop-daemon and supervise-daemon helpers in the declarative context is truly remarkable. Essentially, all of the applicable variables are subject to unquoted expansion - rendering them vulnerable to word splitting and pathname expansion - with the results then being joined and fed to eval! What could possibly go wrong? A great deal, as it transpires. I have a personal fork of OpenRC that aims to address these issues. It specifically uses xargs to process command_args. That is, after all, what it was designed for. I suppose that I ought to get around to proposing that my changes be included. In the meantime, I cannot personally endorse the use of the declarative style and my avoidance of it is deliberate. Sadly, most developers appear to lack the skills to employ even the non-declarative style in a safe fashion. The current state of the shellinaboxd runscript is merely one of a plethora of examples.
lto_tinderbox has reproduced this issue with version 2.20-r4 - Updating summary.
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=c74c520c8d9bc629c0abdfab8de9b836093615dd commit c74c520c8d9bc629c0abdfab8de9b836093615dd Author: David Seifert <soap@gentoo.org> AuthorDate: 2023-04-30 10:06:03 +0000 Commit: David Seifert <soap@gentoo.org> CommitDate: 2023-04-30 10:06:03 +0000 www-misc/shellinabox: treeclean Closes: https://bugs.gentoo.org/712658 Closes: https://bugs.gentoo.org/777396 Closes: https://bugs.gentoo.org/811192 Closes: https://bugs.gentoo.org/833087 Closes: https://bugs.gentoo.org/891057 Closes: https://bugs.gentoo.org/892089 Closes: https://bugs.gentoo.org/893898 Closes: https://bugs.gentoo.org/902069 Signed-off-by: David Seifert <soap@gentoo.org> profiles/package.mask | 1 - www-misc/shellinabox/Manifest | 1 - www-misc/shellinabox/files/shellinaboxd.conf | 77 ----------------- www-misc/shellinabox/files/shellinaboxd.init | 64 -------------- www-misc/shellinabox/files/shellinaboxd.service | 14 --- www-misc/shellinabox/metadata.xml | 15 ---- www-misc/shellinabox/shellinabox-2.20-r4.ebuild | 108 ------------------------ 7 files changed, 280 deletions(-)
