2023-05-18 3:58 GMT+09:00, HexRabbit <h3xrabbit@xxxxxxxxx>:
> From: Kuan-Ting Chen <h3xrabbit@xxxxxxxxx>
>
> Ensure the context's length is valid (excluding VLAs) before casting the
> pointer to the corresponding structure pointer type, also removed
> redundant check on `len_of_ctxts`.
>
> Signed-off-by: Kuan-Ting Chen <h3xrabbit@xxxxxxxxx>
> ---
> fs/ksmbd/smb2pdu.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 972176bff..83b877254 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -969,18 +969,16 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
> len_of_ctxts = len_of_smb - offset;
>
> while (i++ < neg_ctxt_cnt) {
> - int clen;
> -
> - /* check that offset is not beyond end of SMB */
> - if (len_of_ctxts == 0)
> - break;
> + int clen, ctxt_len;
>
> if (len_of_ctxts < sizeof(struct smb2_neg_context))
> break;
>
> pctx = (struct smb2_neg_context *)((char *)pctx + offset);
> clen = le16_to_cpu(pctx->DataLength);
> - if (clen + sizeof(struct smb2_neg_context) > len_of_ctxts)
> + ctxt_len = clen + sizeof(struct smb2_neg_context);
> +
> + if (ctxt_len > len_of_ctxts)
> break;
>
> if (pctx->ContextType == SMB2_PREAUTH_INTEGRITY_CAPABILITIES) {
> @@ -989,6 +987,9 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn
> *conn,
> if (conn->preauth_info->Preauth_HashId)
> break;
>
> + if (ctxt_len < sizeof(struct smb2_preauth_neg_context))
> + break;
> +
> status = decode_preauth_ctxt(conn,
> (struct smb2_preauth_neg_context *)pctx,
> len_of_ctxts);
> @@ -1000,6 +1001,9 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
> if (conn->cipher_type)
> break;
>
> + if (ctxt_len < sizeof(struct smb2_encryption_neg_context))
You need to consider Ciphers flex-array size to validate ctxt_len. we
can get its size using CipherCount in smb2_encryption_neg_context.
> + break;
> +
> decode_encrypt_ctxt(conn,
> (struct smb2_encryption_neg_context *)pctx,
> len_of_ctxts);
> @@ -1009,6 +1013,9 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
> if (conn->compress_algorithm)
> break;
>
> + if (ctxt_len < sizeof(struct smb2_compression_capabilities_context))
Ditto.
> + break;
> +
> decode_compress_ctxt(conn,
> (struct smb2_compression_capabilities_context *)pctx);
> } else if (pctx->ContextType == SMB2_NETNAME_NEGOTIATE_CONTEXT_ID) {
> @@ -1021,6 +1028,10 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
> } else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES) {
> ksmbd_debug(SMB,
> "deassemble SMB2_SIGNING_CAPABILITIES context\n");
> +
> + if (ctxt_len < sizeof(struct smb2_signing_capabilities))
Ditto.
Thanks.
> + break;
> +
> decode_sign_cap_ctxt(conn,
> (struct smb2_signing_capabilities *)pctx,
> len_of_ctxts);
> --
> 2.25.1
>
>
