Oops, sent the wrong version, meant to send this one instead.
On Mon, Jun 16, 2008 at 1:27 PM, Glenn Durfee <gdurfee@xxxxxxxxxx> wrote:
> 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 }
>
Attachment:
bluetooth.patch.2
Description: Binary data
------------------------------------------------------------------------- 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@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/bluez-devel
