net/bluetooth/cmtp/core.c: potential buffer overrun in session->name

net/bluetooth/cmtp/core.c: potential buffer overrun in session->name

[Colin Ian King]

Hi,

Static analysis with Coverity has detected a potential buffer overrun
with a sprintf into session->name in function cmtp_add_connection in
net/bluetooth/cmtp/core.c

The analysis is as follows:

Out-of-bounds write (OVERRUN)
sprintf_overrun: sprintf will overrun its first argument session->name
which can accommodate 18 bytes. The number of bytes written may be 21
bytes, including the terminating null.

363        sprintf(session->name, "%pMR", &session->bdaddr);

So, %pMR produces can potentially produce 0x............MR\0 (where . is
a hex digit) so this accounts for 21 chars. session->name is defined as:

char name[BTNAMSIZ];

and BTNAMSIZE is:

#define BTNAMSIZ 18

Although an obvious fix is to increase BTNAMSIZE to 21 I'm not sure if
this is actually the correct fix.

Colin

Re: net/bluetooth/cmtp/core.c: potential buffer overrun in session->name

[Marcel Holtmann]

Hi Colin,

> Static analysis with Coverity has detected a potential buffer overrun
> with a sprintf into session->name in function cmtp_add_connection in
> net/bluetooth/cmtp/core.c
> 
> The analysis is as follows:
> 
> Out-of-bounds write (OVERRUN)
> sprintf_overrun: sprintf will overrun its first argument session->name
> which can accommodate 18 bytes. The number of bytes written may be 21
> bytes, including the terminating null.
> 
> 363        sprintf(session->name, "%pMR", &session->bdaddr);
> 
> So, %pMR produces can potentially produce 0x............MR\0 (where . is
> a hex digit) so this accounts for 21 chars. session->name is defined as:
> 
> char name[BTNAMSIZ];
> 
> and BTNAMSIZE is:
> 
> #define BTNAMSIZ 18
> 
> Although an obvious fix is to increase BTNAMSIZE to 21 I'm not sure if
> this is actually the correct fix.

go for it.

Regards

Marcel