Hi Greg, >>>> The L2CAP_CONF_EFS and L2CAP_CONF_RFC messages can be sent from >>>> userspace so their structure sizes need to be checked before parsing >>>> them. >>> >>> this message is confusing me. How can these be send from userspace? >> >> So claimed the original reporter. You have the information in your >> inbox, is it incorrect? > > I am pretty sure he meant that the remote attacker can control it from userspace. This is still a wire protocol and not some socket options. > >>>> >>>> Based on a patch from Ran Menscher. >>>> >>>> Reported-by: Ran Menscher <ran.menscher@xxxxxxxxxxxxxxxxxxx> >>>> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >>>> --- >>>> net/bluetooth/l2cap_core.c | 12 ++++++++---- >>>> 1 file changed, 8 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c >>>> index 93daf94565cf..55e48e6efc2b 100644 >>>> --- a/net/bluetooth/l2cap_core.c >>>> +++ b/net/bluetooth/l2cap_core.c >>>> @@ -3361,7 +3361,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data >>>> break; >>>> >>>> case L2CAP_CONF_RFC: >>>> - if (olen == sizeof(rfc)) >>>> + if ((olen == sizeof(rfc)) && >>>> + (endptr - ptr >= L2CAP_CONF_OPT_SIZE + sizeof(rfc))) >>>> memcpy(&rfc, (void *) val, olen); >>> >>> We don’t do ((x == y) && (..)) actually. Using (x == y && ..) is plenty. >> >> Ick, ok, whatever, you all trust that your brains can remember C >> priority levels, me, I trust ()... >> >> I can fix this up to remove the extra (), but I would like _SOMEONE_ to >> at least validate that this resolves the reported issues… > > I need to reproduce this and then I can tell you. so I think that just checking if (len < 0) break; will just prevent any of these attacks. Since in theory you can also do this via the options, but then you can leak at max 2 octets. I posted a simple patch for this. It would be however good if this gets verified that I understood the issues correctly. Regards Marcel
