Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210511152807.Bluez.v1.1.I6d2ab6907d9a84fa62ac8a39daef5bef7ff545d5@changeid>
In-Reply-To: <20210511152807.Bluez.v1.1.I6d2ab6907d9a84fa62ac8a39daef5bef7ff545d5@changeid>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 11 May 2021 11:04:36 -0700
Message-ID: <CABBYNZ+JiqjRR0p6LSFr8MZ6i2pojDDjmh5X5n8MCRih06WEUQ@mail.gmail.com>
Subject: Re: [Bluez PATCH v1] monitor: Fix possible crash of rfcomm packet
To:     Howard Chung <howardchung@google.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        ChromeOS Bluetooth Upstreaming 
        <chromeos-bluetooth-upstreaming@chromium.org>,
        Yun-Hao Chung <howardchung@chromium.org>,
        Archie Pusaka <apusaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Howard,

On Tue, May 11, 2021 at 12:28 AM Howard Chung <howardchung@google.com> wrote:
>
> From: Yun-Hao Chung <howardchung@chromium.org>
>
> When RFCOMM_TEST_EA returns false, btmon assumes packet data has at
> least 5 bytes long. If that assumption fails, btmon could crash when
> trying to read the next byte.
> This patch fix it by checking the remaining size before reading the last
> byte.
>
> Reviewed-by: apusaka@chromium.org
> ---
>
>  monitor/rfcomm.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
> index 9b88a3440e31..76b1123bb23d 100644
> --- a/monitor/rfcomm.c
> +++ b/monitor/rfcomm.c
> @@ -452,6 +452,9 @@ void rfcomm_packet(const struct l2cap_frame *frame)
>                 hdr.length = GET_LEN16(hdr.length);
>         }
>
> +       if (l2cap_frame->size == 0)
> +               goto fail;
> +

if (!l2cap_frame->size)

>         l2cap_frame_pull(&tmp_frame, l2cap_frame, l2cap_frame->size-1);

Or perhaps we can make l2cap_frame_pull check if it can really pull
the frame and return false if it doesn't just as get_*.

>         if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
> --
> 2.31.1.607.g51e8a6a459-goog
>


-- 
Luiz Augusto von Dentz