Re: [PATCH] Bluetooth: hci_event: potential out of bounds parsing ADV events

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

 



Hi Dan,

On 3/30/19 8:25 AM, Dan Carpenter wrote:
> There is a potential out of bounds if "ev->length" is too high or if the
> number of reports are too many.
> 
> Fixes: c215e9397b00 ("Bluetooth: Process extended ADV report event")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Reviewed-By: Tomas Bortoli <tomasbortoli@xxxxxxxxx>

> ---
> Not tested.  I suck at pointer math, and I don't know why the protocol
> requires a "+ 1".  Please review carefully.
> 
>  net/bluetooth/hci_event.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 609fd6871c5a..ee945b3d12e1 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -5417,6 +5417,7 @@ static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>  	u8 num_reports = skb->data[0];
>  	void *ptr = &skb->data[1];
> +	void *end = &skb->data[skb->len];
>  
>  	hci_dev_lock(hdev);
>  
> @@ -5425,6 +5426,8 @@ static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  		u8 legacy_evt_type;
>  		u16 evt_type;
>  
> +		if (ptr + sizeof(*ev) + ev->length + 1 > end)
> +			break;

Assuming that per each iteration, the while cycle, including the call to
process_adv_report() read up to (sizeof(*ev) + ev->length + 1) bytes,

(I don't understand why the +1, but, anyway..)

If the assumption is correct, then the if condition should be:

if (ptr + sizeof(*ev) + ev->length + 1 >= end)

Because as soon as we try to read from the `end` pointer on, we are
out-of-bound.. the valid skb bytes end at `end-1` (included)

Note that also hci_le_adv_report_evt() is likely to need the same fix!


>  		evt_type = __le16_to_cpu(ev->evt_type);
>  		legacy_evt_type = ext_evt_type_to_legacy(evt_type);
>  		if (legacy_evt_type != LE_ADV_INVALID) {
> 


Kind regards,
Tomas



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux