Re: [PATCH] Bluetooth: stop proccessing malicious adv data

[Pavel Skripkin]

On 11/1/21 10:12, Pavel Skripkin wrote:
> Syzbot reported slab-out-of-bounds read in hci_le_adv_report_evt(). The
> problem was in missing validaion check.
>
> We should check if data is not malicious and we can read next data block.
> If we won't check ptr validness, code can read a way beyond skb->end and
> it can cause problems, of course.
>
> Fixes: e95beb414168 ("Bluetooth: hci_le_adv_report_evt code refactoring")
> Reported-and-tested-by: syzbot+e3fcb9c4f3c2a931dc40@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Pavel Skripkin <paskripkin@xxxxxxxxx>
> ---

Hi, Bluetooth maintainers!

friendly ping :)


If anything is wrong with this one, please, let me know


With regards,
Pavel Skripkin


>   net/bluetooth/hci_event.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 0bca035bf2dc..50d1d62c15ec 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -5780,7 +5780,8 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
>   		struct hci_ev_le_advertising_info *ev = ptr;
>   		s8 rssi;
>   
> -		if (ev->length <= HCI_MAX_AD_LENGTH) {
> +		if (ev->length <= HCI_MAX_AD_LENGTH &&
> +		    ev->data + ev->length <= skb_tail_pointer(skb)) {
>   			rssi = ev->data[ev->length];
>   			process_adv_report(hdev, ev->evt_type, &ev->bdaddr,
>   					   ev->bdaddr_type, NULL, 0, rssi,
> @@ -5790,6 +5791,11 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
>   		}
>   
>   		ptr += sizeof(*ev) + ev->length + 1;
> +
> +		if (ptr > (void *) skb_tail_pointer(skb) - sizeof(*ev)) {
> +			bt_dev_err(hdev, "Malicious advertising data. Stopping processing");
> +			break;
> +		}
>   	}
>   
>   	hci_dev_unlock(hdev);