Subject: SDP payload processing vulnerability
Newsgroups: gmane.linux.bluez.devel
Date: 2008-06-16 20:27:50 GMT (24 weeks, 1 day, 10 hours and 33 minutes ago)
Hi all,
The SDP parsing code blindly trusts string length fields in incoming
SDP packets, exposing reliant applications to over-the-wireless memory
manipulation attacks. An attacker need only send a malformed
response to an SDP query to take advantage of this.
This is most apparent in file bluez-libs-3.30/src/sdp.c, lines 988,
994, 1002 (see below). Also elsewhere in the code where input
pointers are advanced without checking bytes remaining to be parsed.
The root of the problem is that in bluez-libs-3.30/src/sdp.c:1125, the
function sdp_extract_pdu() takes a buffer to parse (in) and a pointer
to a length field (out), but it does not take an incoming length field
(in).
Attached is a patch to fix this issue. Basically I added a
"bytesleft" argument to all of the SDP payload processing routines;
length fields are checked
against the number of remaining bytes to ensure the parser doesn't run
past the end of the packet, or do crazy things like malloc two gigs of
memory. This touches a lot of places, and changes the external API
for SDP payload processing, but I don't see any other way to do this
-- the parser MUST be aware of the incoming packet size in order for
this to be secure.
Glenn
bluez-libs-3.30/src/sdp.c:
972 static sdp_data_t *extract_str(const void *p, int *len)
973 {
974 char *s;
975 int n;
976 sdp_data_t *d = malloc(sizeof(sdp_data_t));
977
978 memset(d, 0, sizeof(sdp_data_t));
979 d->dtd = *(uint8_t *) p;
980 p += sizeof(uint8_t);
981 *len += sizeof(uint8_t);
982
983 switch (d->dtd) {
984 case SDP_TEXT_STR8:
985 case SDP_URL_STR8:
986 n = *(uint8_t *) p; // <-- from the incoming packet
987 p += sizeof(uint8_t);
988 *len += sizeof(uint8_t) + n; // <-- blindly
trusted here, may advance parser past end of packet
989 break;
990 case SDP_TEXT_STR16:
991 case SDP_URL_STR16:
992 n = ntohs(bt_get_unaligned((uint16_t *) p)); //
<-- from the incoming packet
993 p += sizeof(uint16_t);
994 *len += sizeof(uint16_t) + n; // <-- blindly
trusted here, may advance parser past end of packet
995 break;
996 default:
997 SDPERR("Sizeof text string > UINT16_MAX\n");
998 free(d);
999 return 0;
1000 }
1001
1002 s = malloc(n + 1); // <-- really blindly trusted here,
also no NULL checking
1003 memset(s, 0, n + 1);
1004 memcpy(s, p, n);
1005
1006 SDPDBG("Len : %d\n", n);
1007 SDPDBG("Str : %s\n", s);
1008
1009 d->val.str = s;
1010 d->unitSize = n + sizeof(uint8_t); // <-- more blind trust
1011 return d;
1012 }
------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php
_______________________________________________ Bluez-devel mailing list Bluez-devel <at> lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bluez-devel