Bug 33431

Summary:	CVE-2011-0523 CVE-2011-0524: arbitrary file access and buffer overflows
Product:	Gypsy	Reporter:	Kees Cook <kees>
Component:	General	Assignee:	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.