incorrect access checks for I2C devices
ddcutil uses access(2) to check that the relevant I²C devices exist and that its process has permission to write to them:
https://github.com/rockowitz/ddcutil/blob/618dbd8b4302f3a8eed66229faf69dd92d4a7ba1/src/app_ddcutil/main.c#L335
https://github.com/rockowitz/ddcutil/blob/618dbd8b4302f3a8eed66229faf69dd92d4a7ba1/src/app_ddcutil/main.c#L395
This is a time-of-check to time-of-use race condition, but more importantly, access() does not actually determine if the calling process can access the file, but if the calling user can access the file. From the man page:
The check is done using the calling process's real UID and GID, rather than the effective IDs as is done when actually attempting an operation (e.g., open(2)) on the file. Similarly, for the root user, the check uses the set of permitted capabilities rather than the set of effective capabilities; and for non-root users, the check uses an empty set of capabilities.
This allows set-user-ID programs and capability-endowed programs to easily determine the invoking user's authority. In other words, access() does not answer the "can I read/write/execute this file?" question. It answers a slightly different question: "(assuming I'm a setuid binary) can the user who invoked me read/write/execute this file?", which gives set-user-ID programs the possibility to prevent malicious users from causing them to read files which users shouldn't be able to read.
(Emphasis mine.)
This prevents using capabilities or setuid to allow non-root users to use ddcutil. ddcutil should instead simply use open() to determine if it can actually do so.
Thank you for the clear discussion of the issue. I will have to think through the implications.
If what you're trying to do is give non-root users the ability to use ddcutil, the udev rule installed by ddcutil uses tag uaccess to give the logged on user RW permission for ddcutil. Does this not address your use case?
@rockowitz The uaccess tag is excellent for the use case of using ddcutil while sitting in front of your computer, but (intentionally) doesn't cover remote access cases. For my case I need to switch monitor inputs sometimes before the user has logged on locally at all. I can of course just run ddcutil as root, which is what I'm currently doing, but I would prefer to not have to.
It's also really useful for testing purposes.
How can you run ddcutil without being logged on as some user? Or are you running without the udev rules having been executed?
I have put a set of changes into branch 2.2.1-dev that bypass use of access() if running as root with the setuid bit set. Does this address your need?
How can you run ddcutil without being logged on as some user? Or are you running without the udev rules having been executed?
You can be logged on as a user without being logged on locally — uaccess is specifically for the local seat. Automated and remote processes are intentionally not covered by uaccess.
I have put a set of changes into branch 2.2.1-dev that bypass use of access() if running as root with the setuid bit set. Does this address your need?
Not as I intended, as I intended to use capabilities(7) for my use case rather than setuid, to limit the privileges given.
Given the imminence of its release, I'm afraid no further changes can be made in release 2.2.1 regarding this enhancement request. I'll keep it in mind for the next release.