2023-05-31 16:35 GMT+09:00, 張智諺 <cc85nod@xxxxxxxxx>:
> I think the root cause of this bug likes the d6c43f4 one.
>
> The attacker can forge ProtocolID to be 0 and bypass the `pdu_size <
> SMB2_MIN_SUPPORTED_HEADER_SIZE` check. When the ksmbd accesses
> `rcv_hdr->Signature` in the function `init_smb2_rsp_hdr`, the oob read bug
> will be triggered.
Thanks for your report:)
Could you please check if this patch fix this issue ?
>From a931b1fd4e186a7abced51c6895914b38970a15d Mon Sep 17 00:00:00 2001
From: Namjae Jeon <linkinjeon@xxxxxxxxxx>
Date: Wed, 31 May 2023 17:59:32 +0900
Subject: [PATCH] ksmbd: validate smb request protocol id
This patch add the validation for smb request protocol id.
If it is not one of the four ids(SMB1_PROTO_NUMBER, SMB2_PROTO_NUMBER,
SMB2_TRANSFORM_PROTO_NUM, SMB2_COMPRESSION_TRANSFORM_ID), don't allow
processing the request. And this will fix the following KASAN warning
also.
[ 13.905265] BUG: KASAN: slab-out-of-bounds in init_smb2_rsp_hdr+0x1b9/0x1f0
[ 13.905900] Read of size 16 at addr ffff888005fd2f34 by task kworker/0:2/44
...
[ 13.908553] Call Trace:
[ 13.908793] <TASK>
[ 13.908995] dump_stack_lvl+0x33/0x50
[ 13.909369] print_report+0xcc/0x620
[ 13.910870] kasan_report+0xae/0xe0
[ 13.911519] kasan_check_range+0x35/0x1b0
[ 13.911796] init_smb2_rsp_hdr+0x1b9/0x1f0
[ 13.912492] handle_ksmbd_work+0xe5/0x820
Reported-by: Chih-Yen Chang <cc85nod@xxxxxxxxx>
Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
---
fs/smb/server/connection.c | 5 +++--
fs/smb/server/smb_common.c | 14 +++++++++++++-
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/fs/smb/server/connection.c b/fs/smb/server/connection.c
index e11d4a1e63d7..2a717d158f02 100644
--- a/fs/smb/server/connection.c
+++ b/fs/smb/server/connection.c
@@ -364,8 +364,6 @@ int ksmbd_conn_handler_loop(void *p)
break;
memcpy(conn->request_buf, hdr_buf, sizeof(hdr_buf));
- if (!ksmbd_smb_request(conn))
- break;
/*
* We already read 4 bytes to find out PDU size, now
@@ -383,6 +381,9 @@ int ksmbd_conn_handler_loop(void *p)
continue;
}
+ if (!ksmbd_smb_request(conn))
+ break;
+
if (((struct smb2_hdr *)smb2_get_msg(conn->request_buf))->ProtocolId ==
SMB2_PROTO_NUMBER) {
if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
index c6e4d38319df..a7e81067bc99 100644
--- a/fs/smb/server/smb_common.c
+++ b/fs/smb/server/smb_common.c
@@ -158,7 +158,19 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
*/
bool ksmbd_smb_request(struct ksmbd_conn *conn)
{
- return conn->request_buf[0] == 0;
+ __le32 *proto = (__le32 *)smb2_get_msg(conn->request_buf);
+
+ if (*proto == SMB2_COMPRESSION_TRANSFORM_ID) {
+ pr_err_ratelimited("smb2 compression not support yet");
+ return false;
+ }
+
+ if (*proto != SMB1_PROTO_NUMBER &&
+ *proto != SMB2_PROTO_NUMBER &&
+ *proto != SMB2_TRANSFORM_PROTO_NUM)
+ return false;
+
+ return true;
}
static bool supported_protocol(int idx)
--
2.25.1
>
> [ 13.905265] BUG: KASAN: slab-out-of-bounds in
> init_smb2_rsp_hdr+0x1b9/0x1f0
> [ 13.905900] Read of size 16 at addr ffff888005fd2f34 by task
> kworker/0:2/44
> ...
> [ 13.908553] Call Trace:
> [ 13.908793] <TASK>
> [ 13.908995] dump_stack_lvl+0x33/0x50
> [ 13.909369] print_report+0xcc/0x620
> [ 13.910870] kasan_report+0xae/0xe0
> [ 13.911519] kasan_check_range+0x35/0x1b0
> [ 13.911796] init_smb2_rsp_hdr+0x1b9/0x1f0
> [ 13.912492] handle_ksmbd_work+0xe5/0x820
> ...
> [ 13.915753] </TASK>
>
From a931b1fd4e186a7abced51c6895914b38970a15d Mon Sep 17 00:00:00 2001
From: Namjae Jeon <linkinjeon@xxxxxxxxxx>
Date: Wed, 31 May 2023 17:59:32 +0900
Subject: [PATCH] ksmbd: validate smb request protocol id
This patch add the validation for smb request protocol id.
If it is not one of the four ids(SMB1_PROTO_NUMBER, SMB2_PROTO_NUMBER,
SMB2_TRANSFORM_PROTO_NUM, SMB2_COMPRESSION_TRANSFORM_ID), don't allow
processing the request. And this will fix the following KASAN warning
also.
[ 13.905265] BUG: KASAN: slab-out-of-bounds in init_smb2_rsp_hdr+0x1b9/0x1f0
[ 13.905900] Read of size 16 at addr ffff888005fd2f34 by task kworker/0:2/44
...
[ 13.908553] Call Trace:
[ 13.908793] <TASK>
[ 13.908995] dump_stack_lvl+0x33/0x50
[ 13.909369] print_report+0xcc/0x620
[ 13.910870] kasan_report+0xae/0xe0
[ 13.911519] kasan_check_range+0x35/0x1b0
[ 13.911796] init_smb2_rsp_hdr+0x1b9/0x1f0
[ 13.912492] handle_ksmbd_work+0xe5/0x820
Reported-by: Chih-Yen Chang <cc85nod@xxxxxxxxx>
Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
---
fs/smb/server/connection.c | 5 +++--
fs/smb/server/smb_common.c | 14 +++++++++++++-
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/fs/smb/server/connection.c b/fs/smb/server/connection.c
index e11d4a1e63d7..2a717d158f02 100644
--- a/fs/smb/server/connection.c
+++ b/fs/smb/server/connection.c
@@ -364,8 +364,6 @@ int ksmbd_conn_handler_loop(void *p)
break;
memcpy(conn->request_buf, hdr_buf, sizeof(hdr_buf));
- if (!ksmbd_smb_request(conn))
- break;
/*
* We already read 4 bytes to find out PDU size, now
@@ -383,6 +381,9 @@ int ksmbd_conn_handler_loop(void *p)
continue;
}
+ if (!ksmbd_smb_request(conn))
+ break;
+
if (((struct smb2_hdr *)smb2_get_msg(conn->request_buf))->ProtocolId ==
SMB2_PROTO_NUMBER) {
if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
index c6e4d38319df..a7e81067bc99 100644
--- a/fs/smb/server/smb_common.c
+++ b/fs/smb/server/smb_common.c
@@ -158,7 +158,19 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
*/
bool ksmbd_smb_request(struct ksmbd_conn *conn)
{
- return conn->request_buf[0] == 0;
+ __le32 *proto = (__le32 *)smb2_get_msg(conn->request_buf);
+
+ if (*proto == SMB2_COMPRESSION_TRANSFORM_ID) {
+ pr_err_ratelimited("smb2 compression not support yet");
+ return false;
+ }
+
+ if (*proto != SMB1_PROTO_NUMBER &&
+ *proto != SMB2_PROTO_NUMBER &&
+ *proto != SMB2_TRANSFORM_PROTO_NUM)
+ return false;
+
+ return true;
}
static bool supported_protocol(int idx)
--
2.25.1
