| Message ID | CAGvGhF6Vf7vmd2vRC6Vv22ZNoxbhX4ym3_NjjiOncvx=ay9X8w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ksmbd: missing validation of hdr->next_offset | expand |
2021-09-21 14:02 GMT+09:00, Leif Sahlberg <lsahlber@redhat.com>: > List, Namjae, Hi Ronnie, > > I have been looking at next_offset and we need some additional > validation of when we > walk to the next pdu without validating we have enough buffer. > > I think the problem is down in is_chained_smb2_message()/ > init_chained_smb2_rsp() > where it advances work->next_smb2_rcv_hdr_off without checking that > the new offset is valid and contains a smb2 header and payload. > > > A simple reproducer is libsmb2. > Apply this patch to break "next_offset" for compounds in libsmb2 : > diff --git a/lib/pdu.c b/lib/pdu.c > index 1329388..4eab8d7 100644 > --- a/lib/pdu.c > +++ b/lib/pdu.c > @@ -174,6 +174,7 @@ smb2_add_compound_pdu(struct smb2_context *smb2, > } > > pdu->header.next_command = offset; > + pdu->header.next_command = 0x0f0ff0f0; > smb2_set_uint32(&pdu->out.iov[0], 20, pdu->header.next_command); > > /* Fixup flags */ > > And then just run this command which uses compounds: > ./examples/smb2-stat-sync smb://192.168.124.221/Share/ > it should oops right away. > > > I don't know where the best place to check for this is, either in > init_chained_smb2_rsp() > but it was a little hard to track it down. > A different place to check this might be from > ksmbd_conn_handler_loop() > and just walk the headers and check next_offset header by header > before even trying to process any of the pdus. Really thanks for your review! I will fix it now. Thanks again! > > > regards > ronnie shalberg > >
diff --git a/lib/pdu.c b/lib/pdu.c index 1329388..4eab8d7 100644 --- a/lib/pdu.c +++ b/lib/pdu.c @@ -174,6 +174,7 @@ smb2_add_compound_pdu(struct smb2_context *smb2, } pdu->header.next_command = offset; + pdu->header.next_command = 0x0f0ff0f0; smb2_set_uint32(&pdu->out.iov[0], 20, pdu->header.next_command); /* Fixup flags */