[Bug 1196085] New: Calling external programs from YaST fragile with full paths
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Bug ID: 1196085 Summary: Calling external programs from YaST fragile with full paths Classification: openSUSE Product: openSUSE Tumbleweed Version: Current Hardware: All OS: Other Status: NEW Severity: Normal Priority: P5 - None Component: YaST2 Assignee: yast2-maintainers@suse.de Reporter: shundhammer@suse.com QA Contact: jsrain@suse.com Found By: --- Blocker: --- After a security audit some years ago we changed all calls to external programs in YaST to use the full path, i.e. from "foobar" to "/sbin/foobar". The rationale was that this would prevent a potential attacker from manipulating the $PATH to include some directory containing a malicious version of the program, i.e. a "/my/hacked/foobar" with $PATH also containing "/my/hacked". With the transition from /bin to /usr/bin and /sbin to /usr/sbin that turned out to be a bad idea since now we need to update those full paths in the YaST code whenever any of those binaries makes that transition, i.e. it's moved from /sbin to /usr/sbin. Worse, if the transition is done in one supported product like Tumbleweed or Leap, yet another one like SLES still uses the old path, we would have to maintain different paths (and not forget to adapt them when that other product makes that transition as well); or we would need to create our own fallback logic to search all the possible paths which one to invoke. But that fallback logic exists, and if handled correctly, it's just as secure as using a full path; but more flexible and not fragile against tools being moved from one system location to another: It's $PATH. We need to make sure that $PATH only contains directories that are not writeable by non-privileged users, and of course not "." (the current directory) or any path that starts with "."; e.g. export PATH="/usr/sbin:/usr/bin:/sbin:/bin" And we already do that in YaST code. So we should migrate calls to external programs back to calling them without the full path and use our secure $PATH instead. For a more in-depth discussion, see also https://github.com/yast/yast-iscsi-client/issues/111 -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 https://bugzilla.suse.com/show_bug.cgi?id=1196085#c1 --- Comment #1 from Stefan Hundhammer <shundhammer@suse.com> --- Specific bug for the iSCSI tools: bug #1196086 -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Stefan Hundhammer <shundhammer@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Calling external programs |Calling external programs |from YaST fragile with full |from YaST is fragile with |paths |full paths -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Stefan Hundhammer <shundhammer@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aschnell@suse.com -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 https://bugzilla.suse.com/show_bug.cgi?id=1196085#c2 --- Comment #2 from Ladislav Slez�k <lslezak@suse.com> --- (In reply to Stefan Hundhammer from comment #0)
And we already do that in YaST code.
Just for the reference, YaST does export PATH=/sbin:/usr/sbin:$PATH https://github.com/yast/yast-yast2/blob/6a14acda1b94e98da6659896e07b73de96fc... -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Ladislav Slez�k <lslezak@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 - None |P3 - Medium Status|NEW |CONFIRMED CC| |lslezak@suse.com -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Ladislav Slez�k <lslezak@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- URL| |https://trello.com/c/I5pScJ | |PL Assignee|yast2-maintainers@suse.de |yast-internal@suse.de -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Lukas Ocilka <locilka@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |locilka@suse.com, | |security-team@suse.de -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 https://bugzilla.suse.com/show_bug.cgi?id=1196085#c3 --- Comment #3 from Ladislav Slez�k <lslezak@suse.com> --- YaST could sanitize the $PATH value like this: ENV["PATH"] = ENV["PATH"].split(":").reject{|p| File.world_writable?(p)}.join(":") -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 https://bugzilla.suse.com/show_bug.cgi?id=1196085#c4 --- Comment #4 from Stefan Hundhammer <shundhammer@suse.com> --- Matthias Gerstner from the security team commented on this at this pull request: https://github.com/yast/yast-iscsi-client/pull/113#issuecomment-1096294103 -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 https://bugzilla.suse.com/show_bug.cgi?id=1196085#c5 --- Comment #5 from Stefan Hundhammer <shundhammer@suse.com> --- See also bug #1204959 -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 https://bugzilla.suse.com/show_bug.cgi?id=1196085#c6 --- Comment #6 from Stefan Hundhammer <shundhammer@suse.com> --- See also bug #1205401 (No /sbin/netconfig) -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 https://bugzilla.suse.com/show_bug.cgi?id=1196085#c7 --- Comment #7 from Stefan Hundhammer <shundhammer@suse.com> --- Knut, I was just looking at the code to fix this on the fly, but then I realized with the latest changes this isn't so easy anymore: https://github.com/yast/yast-ntp-client/pull/174 This changed the full path of the netconfig executable from /sbin to /usr/sbin. That sounds safer, but it's not: We are submitting YaST:Head (i.e. FACTORY) to SLE-15-SP5 (and thus Leap 15.5) as well. But SLE-15-SP5 still uses the OLD executable paths before the usr-merge, i.e. it's still /sbin/netconfig there. Just changing it to /usr/sbin/netconfig doesn't work in that environment since SLE-15 SPx / Leap 15.x don't have that /sbin -> /usr/sbin symlink; for those, ONLY /sbin is the valid path for all those binaries. We agreed some months ago to avoid all those complications by relying on a safe $PATH and calling external binaries from /bin, /sbin, /usr/bin, /usr/sbin without their path, just the binary name: https://github.com/yast/yast-yast2/blob/master/doc/yast-invoking-external-co... But that means that the new check here https://github.com/yast/yast-ntp-client/blob/master/src/modules/NtpClient.rb... def netconfig? FileUtils.Exists(NETCONFIG_PATH) end also doesn't work that easily anymore; we'll have to check for the binary in both possible locations, /usr/sbin and /sbin; or, more generally, in all of $PATH. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 https://bugzilla.suse.com/show_bug.cgi?id=1196085#c8 --- Comment #8 from Stefan Hundhammer <shundhammer@suse.com> --- And while we are at it, maybe migrate that external call from the aging SCR::Execute(.target.bash, ...) to the new Yast::Execute.on_target!(). -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Stefan Hundhammer <shundhammer@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kanderssen@suse.com -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Stefan Hundhammer <shundhammer@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(kanderssen@suse.c | |om) -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 https://bugzilla.suse.com/show_bug.cgi?id=1196085#c9 --- Comment #9 from Stefan Hundhammer <shundhammer@suse.com> --- Knut, what do you think about this? -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Stefan Hundhammer <shundhammer@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(kanderssen@suse.c | |om) | -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1196085 Stefan Hundhammer <shundhammer@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|kanderssen@suse.com | -- You are receiving this mail because: You are on the CC list for the bug.
participants (1)
-
bugzilla_noreply@suse.com