SDP payload processing vulnerability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
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

[Index of Archives]     [Linux Bluetooth Devel]     [Linux USB Devel]     [Network Devel]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux