* [PATCH BlueZ] Fix memory corruption when decoding Read Response PDU
@ 2011-09-28 19:01 Anderson Lizardo
2011-10-03 8:51 ` Johan Hedberg
0 siblings, 1 reply; 4+ messages in thread
From: Anderson Lizardo @ 2011-09-28 19:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
A bogus (or hostile) Proximity Reporter device may send a TX Power value
bigger than the buffer used. Therefore, create a temporary buffer with
the maximum size, and check for the length before using the value.
Note that all other current users of the dec_read_resp() already do
this. Another option would be to change dec_read_resp() to accept a
buffer length, but this would break external code, so it is avoided for
now.
---
proximity/monitor.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/proximity/monitor.c b/proximity/monitor.c
index 0ce48db..884e66d 100644
--- a/proximity/monitor.c
+++ b/proximity/monitor.c
@@ -186,7 +186,7 @@ static int write_alert_level(struct monitor *monitor)
static void tx_power_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
gpointer user_data)
{
- uint8_t value;
+ uint8_t value[ATT_MAX_MTU];
int vlen;
if (status != 0) {
@@ -194,12 +194,17 @@ static void tx_power_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
return;
}
- if (!dec_read_resp(pdu, plen, &value, &vlen)) {
+ if (!dec_read_resp(pdu, plen, value, &vlen)) {
DBG("Protocol error");
return;
}
- DBG("Tx Power Level: %02x", (int8_t) value);
+ if (vlen != 1) {
+ DBG("Invalid length for TX Power value: %d", vlen);
+ return;
+ }
+
+ DBG("Tx Power Level: %02x", (int8_t) value[0]);
}
static void tx_power_handle_cb(GSList *characteristics, guint8 status,
--
1.7.0.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH BlueZ] Fix memory corruption when decoding Read Response PDU
2011-09-28 19:01 [PATCH BlueZ] Fix memory corruption when decoding Read Response PDU Anderson Lizardo
@ 2011-10-03 8:51 ` Johan Hedberg
2011-10-03 22:59 ` Anderson Lizardo
0 siblings, 1 reply; 4+ messages in thread
From: Johan Hedberg @ 2011-10-03 8:51 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
Hi Lizardo,
On Wed, Sep 28, 2011, Anderson Lizardo wrote:
> A bogus (or hostile) Proximity Reporter device may send a TX Power value
> bigger than the buffer used. Therefore, create a temporary buffer with
> the maximum size, and check for the length before using the value.
>
> Note that all other current users of the dec_read_resp() already do
> this. Another option would be to change dec_read_resp() to accept a
> buffer length, but this would break external code, so it is avoided for
> now.
> ---
> proximity/monitor.c | 11 ++++++++---
> 1 files changed, 8 insertions(+), 3 deletions(-)
Applied. Thanks.
Have you considered changing the API so that the caller could tell the
function the size of the supplied buffer?
Another thing (though unrelated to this patch) I noticed: whenever you
have variables that denote some kind of size and are not directly bound
to fixed-length fields in PDU's, please use size_t or ssize_t. Feel free
to send patches to fix such issues in your code.
Johan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH BlueZ] Fix memory corruption when decoding Read Response PDU
2011-10-03 8:51 ` Johan Hedberg
@ 2011-10-03 22:59 ` Anderson Lizardo
2011-10-05 10:32 ` Johan Hedberg
0 siblings, 1 reply; 4+ messages in thread
From: Anderson Lizardo @ 2011-10-03 22:59 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth; +Cc: Vinicius Gomes, Claudio Takahasi
Hi Johan,
On Mon, Oct 3, 2011 at 10:51 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lizardo,
>
> On Wed, Sep 28, 2011, Anderson Lizardo wrote:
>> A bogus (or hostile) Proximity Reporter device may send a TX Power value
>> bigger than the buffer used. Therefore, create a temporary buffer with
>> the maximum size, and check for the length before using the value.
>>
>> Note that all other current users of the dec_read_resp() already do
>> this. Another option would be to change dec_read_resp() to accept a
>> buffer length, but this would break external code, so it is avoided for
>> now.
>> ---
>> proximity/monitor.c | 11 ++++++++---
>> 1 files changed, 8 insertions(+), 3 deletions(-)
>
> Applied. Thanks.
>
> Have you considered changing the API so that the caller could tell the
> function the size of the supplied buffer?
Yes. I even mentioned on the last paragraph :) I was afraid it was too
late to change this, but if you thing it is not an issue for
external/third party code to break now, we can change this now.
> Another thing (though unrelated to this patch) I noticed: whenever you
> have variables that denote some kind of size and are not directly bound
> to fixed-length fields in PDU's, please use size_t or ssize_t. Feel free
> to send patches to fix such issues in your code.
This can be fixed, I think. We used "int" and guint* types at some
parts, but I don't remember if there was any rationale.
Claudio, Vinicius: can we get this easily fixed on attrib/* code?
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH BlueZ] Fix memory corruption when decoding Read Response PDU
2011-10-03 22:59 ` Anderson Lizardo
@ 2011-10-05 10:32 ` Johan Hedberg
0 siblings, 0 replies; 4+ messages in thread
From: Johan Hedberg @ 2011-10-05 10:32 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth, Vinicius Gomes, Claudio Takahasi
Hi Lizardo,
On Tue, Oct 04, 2011, Anderson Lizardo wrote:
> > Have you considered changing the API so that the caller could tell the
> > function the size of the supplied buffer?
>
> Yes. I even mentioned on the last paragraph :)
Yep, I missed it. Sorry.
> I was afraid it was too late to change this, but if you thing it is
> not an issue for external/third party code to break now, we can change
> this now.
This is not a public API so there's no problem there. (even if it was
public we'd have marked it as experimental for now)
Johan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-10-05 10:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-28 19:01 [PATCH BlueZ] Fix memory corruption when decoding Read Response PDU Anderson Lizardo
2011-10-03 8:51 ` Johan Hedberg
2011-10-03 22:59 ` Anderson Lizardo
2011-10-05 10:32 ` Johan Hedberg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.