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

From: Colin Ian King <colin.king@canonical.com>
To: Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: linux-bluetooth@vger.kernel.org
Subject: net/bluetooth/cmtp/core.c: potential buffer overrun in session->name
Date: Wed, 4 Aug 2021 10:08:18 +0100	[thread overview]
Message-ID: <2cfa6b86-da6a-207f-ec1a-e180e6275896@canonical.com> (raw)

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