2021-10-18 23:49 GMT+09:00, Marios Makassikis <mmakassikis@xxxxxxxxxx>: > On Mon, Oct 18, 2021 at 4:04 PM Namjae Jeon <linkinjeon@xxxxxxxxxx> wrote: >> >> Hi Marios, >> > + negblob_off = le16_to_cpu(req->SecurityBufferOffset); >> > + negblob_len = le16_to_cpu(req->SecurityBufferLength); >> > + if (negblob_off < (offsetof(struct smb2_sess_setup_req, Buffer) - >> > 4)) >> > + return -EINVAL; >> Like the following code, negblob is still used without buffer check. >> We need to add buffer check for it here ? >> >> if (negblob->MessageType == NtLmNegotiate) { >> >> } else if (negblob->MessageType == NtLmAuthenticate) { >> >> Thanks! >> > Hello Namjae, > > I'm not sure I understand what you mean. Should I change the check to > something like this ? > > + negblob_off = le16_to_cpu(req->SecurityBufferOffset); > + negblob_len = le16_to_cpu(req->SecurityBufferLength); > + if (negblob_off < (offsetof(struct smb2_sess_setup_req, Buffer) - 4) > || > + negblob_len < sizeof(struct negotiate_message)) negblob_len < offsetof(struct negotiate_message, NegotiateFlags)) Thanks! > > Marios > >> > + >> > negblob = (struct negotiate_message *)((char >> > *)&req->hdr.ProtocolId + >> > - le16_to_cpu(req->SecurityBufferOffset)); >> > + negblob_off); >> > >> > - if (decode_negotiation_token(work, negblob) == 0) { >> > + if (decode_negotiation_token(conn, negblob, negblob_len) == 0) { >> > if (conn->mechToken) >> > negblob = (struct negotiate_message >> > *)conn->mechToken; >> > } >> > @@ -1736,7 +1746,7 @@ int smb2_sess_setup(struct ksmbd_work *work) >> > sess->Preauth_HashValue = NULL; >> > } else if (conn->preferred_auth_mech == >> > KSMBD_AUTH_NTLMSSP) { >> > if (negblob->MessageType == NtLmNegotiate) { >> > - rc = ntlm_negotiate(work, negblob); >> > + rc = ntlm_negotiate(work, negblob, >> > negblob_len); >> > if (rc) >> > goto out_err; >> > rsp->hdr.Status = >> > -- >> > 2.25.1 >> > >> > >