bluetooth: Add cmd validity checks at the start of hci_sock_ioctl()

Commit Message

Ruihan Li April 16, 2023, 8:02 a.m. UTC

Previously, channel open messages were always sent to monitors on the first
ioctl() call for unbound HCI sockets, even if the command and arguments
were completely invalid. This can leave an exploitable hole with the abuse
of invalid ioctl calls.

This commit hardens the ioctl processing logic by first checking if the
command is valid, and immediately returning with an ENOIOCTLCMD error code
if it is not. This ensures that ioctl calls with invalid commands are free
of side effects, and increases the difficulty of further exploitation by
forcing exploitation to find a way to pass a valid command first.

Signed-off-by: Ruihan Li <[e-mail]>
Co-developed-by: Marcel Holtmann <[e-mail]>
Signed-off-by: Marcel Holtmann <[e-mail]>
---
 net/bluetooth/hci_sock.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

[e-mail] April 16, 2023, 8:34 a.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=740179

---Test result---

Test Summary:
CheckPatch                    PASS      0.68 seconds
GitLint                       PASS      0.34 seconds
SubjectPrefix                 FAIL      0.37 seconds
BuildKernel                   PASS      30.78 seconds
CheckAllWarning               PASS      34.52 seconds
CheckSparse                   PASS      38.40 seconds
CheckSmatch                   PASS      108.39 seconds
BuildKernel32                 PASS      29.87 seconds
TestRunnerSetup               PASS      429.50 seconds
TestRunner_l2cap-tester       PASS      16.30 seconds
TestRunner_iso-tester         PASS      19.52 seconds
TestRunner_bnep-tester        PASS      5.19 seconds
TestRunner_mgmt-tester        PASS      109.79 seconds
TestRunner_rfcomm-tester      PASS      8.44 seconds
TestRunner_sco-tester         PASS      7.78 seconds
TestRunner_ioctl-tester       PASS      8.91 seconds
TestRunner_mesh-tester        PASS      6.58 seconds
TestRunner_smp-tester         PASS      7.58 seconds
TestRunner_userchan-tester    PASS      5.46 seconds
IncrementalBuild              PASS      28.20 seconds

Details
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject


---
Regards,
Linux Bluetooth

[e-mail] April 17, 2023, 6:30 p.m. UTC | #2

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[e-mail]>:

On Sun, 16 Apr 2023 16:02:51 +0800 you wrote:
> Previously, channel open messages were always sent to monitors on the first
> ioctl() call for unbound HCI sockets, even if the command and arguments
> were completely invalid. This can leave an exploitable hole with the abuse
> of invalid ioctl calls.
> 
> This commit hardens the ioctl processing logic by first checking if the
> command is valid, and immediately returning with an ENOIOCTLCMD error code
> if it is not. This ensures that ioctl calls with invalid commands are free
> of side effects, and increases the difficulty of further exploitation by
> forcing exploitation to find a way to pass a valid command first.
> 
> [...]

Here is the summary with links:
  - bluetooth: Add cmd validity checks at the start of hci_sock_ioctl()
    https://git.kernel.org/bluetooth/bluetooth-next/c/5612e6a8ff35

You are awesome, thank you!

Patch

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index f597fe0db..1d249d839 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -987,6 +987,34 @@  static int hci_sock_ioctl(struct socket *sock, unsigned int cmd,
 
 	BT_DBG("cmd %x arg %lx", cmd, arg);
 
+	/* Make sure the cmd is valid before doing anything */
+	switch (cmd) {
+	case HCIGETDEVLIST:
+	case HCIGETDEVINFO:
+	case HCIGETCONNLIST:
+	case HCIDEVUP:
+	case HCIDEVDOWN:
+	case HCIDEVRESET:
+	case HCIDEVRESTAT:
+	case HCISETSCAN:
+	case HCISETAUTH:
+	case HCISETENCRYPT:
+	case HCISETPTYPE:
+	case HCISETLINKPOL:
+	case HCISETLINKMODE:
+	case HCISETACLMTU:
+	case HCISETSCOMTU:
+	case HCIINQUIRY:
+	case HCISETRAW:
+	case HCIGETCONNINFO:
+	case HCIGETAUTHINFO:
+	case HCIBLOCKADDR:
+	case HCIUNBLOCKADDR:
+		break;
+	default:
+		return -ENOIOCTLCMD;
+	}
+
 	lock_sock(sk);
 
 	if (hci_pi(sk)->channel != HCI_CHANNEL_RAW) {