smbd_do_qfilepathinfo should respect max_data_bytes when responding to a QUERY_FILE_STREAMINFO request

[Richard Sharpe]

On Mon, Dec 3, 2012 at 6:08 PM, Richard Sharpe
<realrichardsharpe at gmail.com> wrote:
> Hi Folks,
>
> This bug causes at least Win 7 to fail to display stream info. Real
> Windows returns STATUS_BUFFER_OVERFLOW, at least with SMB2 when the
> stream info is larger than the allowed size. There also seems to be
> some leakage there as well.
>
> index 61d755c..0e41140 100644
> --- a/source3/smbd/trans2.c
> +++ b/source3/smbd/trans2.c
> @@ -4801,9 +4801,19 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
>                         if (!NT_STATUS_IS_OK(status)) {
>                                 DEBUG(10, ("marshall_stream_info failed: %s\n",
>                                            nt_errstr(status)));
> +                               TALLOC_FREE(streams);
>                                 return status;
>                         }
>
> +                       /*
> +                        * We should return STATUS_BUFFER_OVERFLOW if there is
> +                        * not enough space to return the data.
> +                        */
> +                       if (data_size > max_data_bytes) {
> +                               TALLOC_FREE(streams);
> +                               return STATUS_BUFFER_OVERFLOW;
> +                       }
> +
>                         TALLOC_FREE(streams);
>
>                         break;
>

OK, that is not the correct fix. This is a better fix. I will test it
out today and see. There is a buffer overflow happening in there at
the moment as well, I believe.

index 61d755c..9bd1f53 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -4159,7 +4159,7 @@ static NTSTATUS marshall_stream_info(unsigned int num_stre
        unsigned int i;
        unsigned int ofs = 0;

-       for (i = 0; i < num_streams && ofs <= max_data_bytes; i++) {
+       for (i = 0; i < num_streams; i++) {
                unsigned int next_offset;
                size_t namelen;
                smb_ucs2_t *namebuf;
@@ -4178,6 +4178,15 @@ static NTSTATUS marshall_stream_info(unsigned int num_str

                namelen -= 2;

+               /*
+                * We cannot overflow ...
+                */
+               if ((ofs + 24 + namelen) > max_data_bytes) {
+                       DEBUG(10, ("overflowed the data buffer at stream %u\n",
+                               i));
+                       return STATUS_BUFFER_OVERFLOW;
+               }
+
                SIVAL(data, ofs+4, namelen);
                SOFF_T(data, ofs+8, streams[i].size);
                SOFF_T(data, ofs+16, streams[i].alloc_size);
@@ -4801,6 +4810,7 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
                        if (!NT_STATUS_IS_OK(status)) {
                                DEBUG(10, ("marshall_stream_info failed: %s\n",
                                           nt_errstr(status)));
+                               TALLOC_FREE(streams);
                                return status;
                        }



-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)