Bug 33431

Summary: CVE-2011-0523 CVE-2011-0524: arbitrary file access and buffer overflows
Product: Gypsy Reporter: Kees Cook <kees>
Component: GeneralAssignee: Ross Burton <ross>
Status: RESOLVED FIXED QA Contact:
Severity: critical    
Priority: medium CC: bugzilla, pbrobinson
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: security fix
Fix buffer overflows in 0.8 codebase

Description Kees Cook 2011-01-24 13:54:52 UTC
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.)
Comment 1 Bastien Nocera 2011-10-12 03:58:01 UTC
Created attachment 52255 [details] [review]
security fix

Patch by Michael Leibowitz, further modified by myself.
Comment 2 Bastien Nocera 2011-10-12 04:17:12 UTC
Created attachment 52256 [details] [review]
Fix buffer overflows in 0.8 codebase
Comment 3 Bastien Nocera 2011-10-12 04:22:11 UTC
(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.
Comment 4 iain 2011-10-12 07:36:26 UTC
Now that we have the discovery system in master, we could use that to only allow known device files to be connected to
Comment 5 Bastien Nocera 2011-10-13 02:11:10 UTC
(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).
Comment 6 Bastien Nocera 2011-10-31 06:50:25 UTC
Could I get some review on those patches?
Comment 7 Bastien Nocera 2012-04-16 08:39:22 UTC
Again, can I get a review on those patches, please?
Comment 8 Kees Cook 2012-04-17 13:32:31 UTC
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.
Comment 9 iain 2012-05-14 07:31:04 UTC
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.