Re: There is an array-index-out-bounds bug in detach_capi_ctr in drivers/isdn/capi/kcapi.c

[butt3rflyh4ck]

Hi, I make a patch for this issue.

Regards,
 butt3rflyh4ck.


On Fri, Sep 24, 2021 at 6:02 PM butt3rflyh4ck
<butterflyhuangxx@xxxxxxxxx> wrote:
>
> > When I last touched the capi code, I tried to remove it all, but we then
> > left it in the kernel because the bluetooth cmtp code can still theoretically
> > use it.
> >
> > May I ask how you managed to run into this? Did you find the bug through
> > inspection first and then produce it using cmtp, or did you actually use
> > cmtp?
>
> I fuzz the bluez system and find a crash to analyze it and reproduce it.
>
> > If the only purpose of cmtp is now to be a target for exploits, then I
> > would suggest we consider removing both cmtp and capi for
> > good after backporting your fix to stable kernels. Obviously
> > if it turns out that someone actually uses cmtp and/or capi, we
> > should not remove it.
> >
> Yes, I think this should be feasible.
>
> Regards
>   butt3rflyh4ck.
>
>
> --
> Active Defense Lab of Venustech



-- 
Active Defense Lab of Venustech
From 15449bb5aac1f828dc82054651fe8f4dca37bef7 Mon Sep 17 00:00:00 2001
From: Xiaolong Huang <butterflyhuangxx@xxxxxxxxx>
Date: Tue, 28 Sep 2021 13:03:41 +0800
Subject: [PATCH] isdn: cpai: check ctr->cnr to avoid array index out of bound

The cmtp_add_connection() would add a cmtp session to a controller and run a kernel
thread to process cmtp.

	__module_get(THIS_MODULE);
	session->task = kthread_run(cmtp_session, session, "kcmtpd_ctr_%d",
								session->num);

During this process, the kernel thread would call detach_capi_ctr()
to detach a register controller. if the controller was not attached yet, detach_capi_ctr()
would trigger an array-index-out-bounds bug.

[   46.866069][ T6479] UBSAN: array-index-out-of-bounds in
drivers/isdn/capi/kcapi.c:483:21
[   46.867196][ T6479] index -1 is out of range for type 'capi_ctr *[32]'
[   46.867982][ T6479] CPU: 1 PID: 6479 Comm: kcmtpd_ctr_0 Not tainted
5.15.0-rc2+ #8
[   46.869002][ T6479] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.14.0-2 04/01/2014
[   46.870107][ T6479] Call Trace:
[   46.870473][ T6479]  dump_stack_lvl+0x57/0x7d
[   46.870974][ T6479]  ubsan_epilogue+0x5/0x40
[   46.871458][ T6479]  __ubsan_handle_out_of_bounds.cold+0x43/0x48
[   46.872135][ T6479]  detach_capi_ctr+0x64/0xc0
[   46.872639][ T6479]  cmtp_session+0x5c8/0x5d0
[   46.873131][ T6479]  ? __init_waitqueue_head+0x60/0x60
[   46.873712][ T6479]  ? cmtp_add_msgpart+0x120/0x120
[   46.874256][ T6479]  kthread+0x147/0x170
[   46.874709][ T6479]  ? set_kthread_struct+0x40/0x40
[   46.875248][ T6479]  ret_from_fork+0x1f/0x30
[   46.875773][ T6479]

Signed-off-by: Xiaolong Huang <butterflyhuangxx@xxxxxxxxx>
---
 drivers/isdn/capi/kcapi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index cb0afe897162..7313454e403a 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -480,6 +480,11 @@ int detach_capi_ctr(struct capi_ctr *ctr)
 
 	ctr_down(ctr, CAPI_CTR_DETACHED);
 
+	if (ctr->cnr < 1 || ctr->cnr - 1 >= CAPI_MAXCONTR) {
+		err = -EINVAL;
+		goto unlock_out;
+	}
+
 	if (capi_controller[ctr->cnr - 1] != ctr) {
 		err = -EINVAL;
 		goto unlock_out;
-- 
2.25.1