https://bugs.launchpad.net/ubuntu/+source/gypsy/+bug/690323 Regular users can request that arbitrary files be opened for reading. In the best case, this is a denial of service. Worst-case, this could lead to information disclosure or privilege escalation. ** (gypsy-daemon:23540): DEBUG: Creating client for /etc/shadow ** (gypsy-daemon:23540): DEBUG: Device name: shadow ** (gypsy-daemon:23540): DEBUG: Registered client on /org/freedesktop/Gypsy/shadow ** (gypsy-daemon:23540): DEBUG: Starting connection to /etc/shadow ** (gypsy-daemon:23540): DEBUG: Starting connection to /etc/shadow open("/etc/shadow", O_RDONLY|O_NOCTTY|O_NONBLOCK) = 6 open("/etc/shadow", O_RDWR|O_NOCTTY|O_NONBLOCK) = 7 ** (gypsy-daemon:23540): DEBUG: GPS channel can connect There appear to be unchecked buffer overflows as well in gps_channel_garmin_input() via nmeabuf and nmea_gpgsv(), which could be used in an attack. (If the local user attaches gypsy to a pseudo-tty they might be able to trick the string handling.)
Created attachment 52255 [details] [review] security fix Patch by Michael Leibowitz, further modified by myself.
Created attachment 52256 [details] [review] Fix buffer overflows in 0.8 codebase
(In reply to comment #0) <snip> > There appear to be unchecked buffer overflows as well in > gps_channel_garmin_input() via nmeabuf and nmea_gpgsv(), which could be used in > an attack. (If the local user attaches gypsy to a pseudo-tty they might be able > to trick the string handling.) Note that this is only a problem in the 0.8 codebase, the latest master's parsing code is completely rewritten and doesn't use sprintf() anymore.
Now that we have the discovery system in master, we could use that to only allow known device files to be connected to
(In reply to comment #4) > Now that we have the discovery system in master, we could use that to only > allow known device files to be connected to That could be a second keyword for the conf file (bluetooth-known). Though I'd like to say that the current code will hide a lot of older Bluetooth GPS devices because they don't use the correct Class (the positioning device class was fairly recent).
Could I get some review on those patches?
Again, can I get a review on those patches, please?
Buffer overflow fixes look fine to me. I still prefer that udev or something is used to set permissions on devices and that this daemon run as a non-root user. The whitelist is better than nothing, but it'd be nice to see this not running as root at all.
Have committed the first patch. The second patch no longer applies to the codebase as the whole NMEA parser system has been removed and there isn't a 0.8 branch to apply it to.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.