https://bugzilla.redhat.com/show_bug.cgi?id=1394962 --- Comment #10 from Nathaniel McCallum <npmccallum@xxxxxxxxxx> --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #9) > I saw the that one of the files is suid, so I decided to have a look > at the code... > > Two issues pop out: > 1. memset(key, 0, strlen(key)); is going to be optimized away. If you > are doing that to decrease chances of the key leaking out, this will > not work. See > https://github.com/systemd/systemd/blob/ > 493fd52f1ada36bfe63301d4bb50f7fd2b38c670/src/basic/string-\ > util.c#L834 > for a way (cribbed from openssl), for a way around that. > > 2. I don't think using the relative path like as in > snprintf(path, pathl, "%s/../bin/clevis-decrypt", dirname(tmp)) > is safe. If the user is able to call the binary under a different > path where they control the ../bin prefix, it will allow them to call > arbitrary binary as root. The clevis-decrypt binary is not executed as root. We drop privileges much earlier than that. See: https://github.com/latchset/clevis/blob/master/clevis-luks-udisks2.c#L384 However, your concern is still valid because we pass information obtained as root to that process. So it still represents a security concern. I'd love to chat with you on IRC to discuss some of my concerns with my own code if you have time. > I don't think it's directly exploitable in > the default configuration, but it's not hard to contrive some scenario > where it would be: > > - fs.protected_hardlinks=0 > This is the kernel default, which is overridden by systemd-sysctl > after boot. If systemd-sysctl is prevented from running, for > example by broken configuration, clevis-luks-udisks2 becomes > immediately exploitable a la CVE-2009-1894. > > - some scenario where the fs containing the suid binary is mounted > below a user writable directory (for example the administrator > temporarily mounts the fs under /var/tmp for backup purposes). > > - some manipulation using mount namespaces. > > I don't see an exploit, but relying on the binary always being visible > only under the correct path seems very brittle. It'd seem much better > to substitute the real absolute path during build. Yeah, I agree. The main reason I haven't done this is because it makes in-tree unit testing (after build, before install) more difficult. Suggestions welcome. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx