In FreeBSD 12.0, basename(3) was changed to be POSIX compliant. This implies that it can possibly write to the passed string. rssh passes a const string, so it always crashes on invocation. There are 2 solutions: * modify basename(3) so it only modifies the string if it has to * modify rssh, making a copy of the string first and pass it instead The first solution may help other ports as well but I know that it will take some time until this change will be public. Meanwhile we could fix at least this single port by making it more POSIX compliant.
Created attachment 201321 [details] basename(3): avoid unnecessary writing to passed string
Created attachment 201322 [details] rssh: pass string copy to basename(3) so it can safely modify it
Comment on attachment 201322 [details] rssh: pass string copy to basename(3) so it can safely modify it This patch looks good. Approved.
Comment on attachment 201321 [details] basename(3): avoid unnecessary writing to passed string There is no basename.c in the extracted work/rssh-2.3.4 directory, and we can't patch basename.c for the kernel, etc., so this won't work. Not approved.
topical, I think you'll want to add your same fix from util.c before all invocations of basename(3): %grep -n basename *.c log.c:96: ident = strdup(basename((char*)name)); rssh_chroot_helper.c:162: progname = strdup(log_make_ident(basename(argv[0]))); util.c:166: char *prog; /* basename of cmd */ util.c:189: prog = basename(cmd); So also in log.c and rssh_chroot_helper.c, right? Thanks!
Tell me when final fix is done and which is the final decision. Then I'll commit it to ports tree.
Created attachment 201752 [details] comply POSIX standard when using basename, part 1
Created attachment 201753 [details] comply POSIX standard when using basename, part 2
Created attachment 201754 [details] comply POSIX standard when using basename, part 3
Created attachment 201755 [details] comply POSIX standard when using basename, part 4
jharris, you are right - there are multiple invocations of basename. I tried to fix them all and created a patch set.
Created attachment 202073 [details] shar patch - supersedes all others This final sharfile of 3 patches follows the original coding style in util.c and _attempts_ to be as clear as possible. It supersedes all earlier patches on this PR. topical, thanks for pointing out the problem with the port on FreeBSD 12 and your initial patches. Tested on: 12.0-RELEASE-p3 GENERIC amd64
(In reply to jharris from comment #12) Just a short question: The shar file seems to contain a bogus file (named "pa" with no content in it)? I left that file intentionally out and with the three other patches (patch-log.c, patch-rssh__chroot__helper.c and patch-util.c) it compiles so far fine on all FreeBSD releases (amd64 + i386). If you can confirm that there's no patch is missing then everything is set to commit these files.
(In reply to Kai Knoblich from comment #13) Yes, ‘pa’ was the original patch file (attached to this bug) and should be removed. Thanks!
Created attachment 202528 [details] rssh_with_basename_and_security_fixes.patch Attached is an updated version for rssh. I did some testing on 11.2-, 12.0-RELEASE and 13.0-CURRENT@r344684. On 13.0-CURRENT shells/rssh didn't work as expected because there was a reference to unallocated memory (as far I can tell with my rudimentary C knowledge). The reason was in util.c: > prog = basename(tmp2); > free (tmp2); > [...] I have fixed that and placed the "free (tmp2)" after the if-contruct when the variable "prog" has been used for the string comparison. The updated patch incorporates also four patches taken from Debian: - 0003-Fix-buffer-allocation-buffer-for-fail-message.patch -> Included in patch-util.c - 0007-Verify-rsync-command-options.patch (updated version with tighter syntax checking) -> Included in optional-patch_util.c - 0009-Verify-scp-command-options.patch -> Included in patch-util.c - 0010-Check-command-line-after-chroot.patch - Included in patch-rssh__chroot__helper.c Patch "0009-Verify-scp-command-options.patch" addresses an actual CVE issue [1]. Then there's fact that upstream has abandoned shells/rssh since some years ago [2] and the software were solely maintained by an individual of Debian. But he will pull the plug for it, so rssh won't be in the next Debian stable release. Thus I think it would be the best to deprecate the port in the FreeBSD ports tree. What do you think? Are there any suggestions/objections? -- [1] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-1000018 [2] https://sourceforge.net/p/rssh/mailman/message/36525645/ [3] https://sourceforge.net/p/rssh/mailman/message/36530715/
Thank you for adding even more patches. At least I use this packet intensively: my switches allow backing up their configuration by scp. So, avoid using scp altogether isn't really an option, but perhaps I can find another packet that does the same job.
I'm going to commit the whole set of patches in some minutes because it's a security concern due the recently reported CVEs. But I'm not adding a deprecation note, yet. I'll let the final decision to the maintainer. @jharris: Would it be okay to set a deprecation date on shells/rssh? If so, what date should be set?
A commit references this bug: Author: kai Date: Wed Mar 6 19:56:57 UTC 2019 New revision: 494835 URL: https://svnweb.freebsd.org/changeset/ports/494835 Log: security/vuxml: Document shells/rssh < 2.3.4_2 vulnerabilities PR: 235121 Approved by: tcberner (mentor) Differential Revision: https://reviews.freebsd.org/D19473 Changes: head/security/vuxml/vuln.xml
A commit references this bug: Author: kai Date: Wed Mar 6 20:45:28 UTC 2019 New revision: 494837 URL: https://svnweb.freebsd.org/changeset/ports/494837 Log: shells/rssh: Apply fixes for basename(3) handling and some security issues basename(3) has been changed to be POSIX compliant in r308264. This implies that it can possibly write to the passed string. shells/rssh passes a const string, so it always crashes on invocation with FreeBSD 12 and later. The new patches remedy this issue. [1] [2] During further tests and research came to light that there were also recently discovered security issues with the parsing of rsync/scp command line arguments and insufficient sanitization of environment variables when using rysnc. The corresponding fixes have been incorporated to the new patches and the already existing patch for the RSYNC option has been tightened for the argument parsing. Please note that with this patch the scp option "-3" can no longer be used. [3] Furthermore, another patch was applied to make this port a bit more secure. That patch handles a buffer allocation issue for an error message. [4] PR: 235121 Submitted by: topical@gmx.net (first version) [1], Jason Harris (maintainer) [2] Approved by: tcberner (mentor) Obtained from: Debian [3] [4] MFH: 2019Q1 Security: d193aa9f-3f8c-11e9-9a24-6805ca0b38e8 Differential Revision: https://reviews.freebsd.org/D19474 Changes: head/shells/rssh/Makefile head/shells/rssh/files/optional-patch-util.c head/shells/rssh/files/patch-log.c head/shells/rssh/files/patch-rssh__chroot__helper.c head/shells/rssh/files/patch-util.c
(In reply to Kai Knoblich from comment #17) Sure, it can be deprecated on whatever schedule is standard for these circumstances. Thanks!
A commit references this bug: Author: kai Date: Thu Mar 7 14:59:37 UTC 2019 New revision: 494951 URL: https://svnweb.freebsd.org/changeset/ports/494951 Log: MFH: r494837 shells/rssh: Apply fixes for basename(3) handling and some security issues basename(3) has been changed to be POSIX compliant in r308264. This implies that it can possibly write to the passed string. shells/rssh passes a const string, so it always crashes on invocation with FreeBSD 12 and later. The new patches remedy this issue. [1] [2] During further tests and research came to light that there were also recently discovered security issues with the parsing of rsync/scp command line arguments and insufficient sanitization of environment variables when using rysnc. The corresponding fixes have been incorporated to the new patches and the already existing patch for the RSYNC option has been tightened for the argument parsing. Please note that with this patch the scp option "-3" can no longer be used. [3] Furthermore, another patch was applied to make this port a bit more secure. That patch handles a buffer allocation issue for an error message. [4] PR: 235121 Submitted by: topical@gmx.net (first version) [1], Jason Harris (maintainer) [2] Approved by: tcberner (mentor) Obtained from: Debian [3] [4] Security: d193aa9f-3f8c-11e9-9a24-6805ca0b38e8 Differential Revision: https://reviews.freebsd.org/D19474 Approved by: ports-secteam (riggs), mentors implicit Changes: _U branches/2019Q1/ branches/2019Q1/shells/rssh/Makefile branches/2019Q1/shells/rssh/files/optional-patch-util.c branches/2019Q1/shells/rssh/files/patch-log.c branches/2019Q1/shells/rssh/files/patch-rssh__chroot__helper.c branches/2019Q1/shells/rssh/files/patch-util.c
A commit references this bug: Author: kai Date: Mon Mar 11 15:59:03 UTC 2019 New revision: 495357 URL: https://svnweb.freebsd.org/changeset/ports/495357 Log: shells/rssh: Mark as deprecated and set to expire Upstream has officially abandoned the port some time ago and the software was maintained then for a while by the Debian project. But even Debian will now pull the plug and rssh won't be available in their next stable release. Considering these facts it makes sense to let this port expire towards the end of Q2. PR: 235121 Approved by: miwi (mentor), maintainer Differential Revision: https://reviews.freebsd.org/D19503 Changes: head/shells/rssh/Makefile
All changes have been committed, thank you all for the patches and feedback!