[Samba] Re: Test Failure for RW1 with samba-3.0.30, Solaris 9

[Jeremy Allison]

On Mon, Jun 02, 2008 at 07:22:09PM -0400, David Eisner wrote:
> Hmm, I wonder if this isn't a bug with the fix for CVE-2008-1105.
> 
> I'll add a bug (to the 717 [1] "NEW" bugs for Samba 3.0 ...), but
> here's what seems to be going on at a low level:
> 
> When the client state is setup in run_readwritetest() by way of
> torture_open_connection(), cli_state->bufsize gets set to
> CLI_SAMBA_MAX_LARGE_READX_SIZE, which is defined to be 127*1024 ==
> 130048.
> 
> During the rw_torture2 iteration that breaks the test,
> send_file_readX() in smbd/reply.c calculates the packet length to
> send:
> 
>     nread = read_file(fsp,data,startpos,smb_maxcnt);
>     // ...
> 
>     outsize = set_message(outbuf,12,nread,False);
>     // ...
> 
>     /* Returning the number of bytes we want to send back - including header. */
>     return outsize;
> }
> 
> When RW1 fails, nread == 130048 (which is
> CLI_SAMBA_MAX_LARGE_READX_SIZE), and outsize is set by set_message()
> to be 39 + 2x12 + nread == 130111.
> 
> Later on, construct_reply reduces this by 4, and I think this becomes
> the length of the reply packet:
> 
>   static int construct_reply(char *inbuf,char *outbuf,int size,int bufsize)
>   {
>       // ...
>       if(outsize > 4)
>           smb_setlen(outbuf,outsize - 4);
>       return(outsize);
>   }
> 
> Now the length stored in the packet is 130111 - 4 == 130107
> 
> Back on the client side, receive_smb_raw()  (in lib/util_sock.c) is unhappy:
> 
>   BOOL receive_smb_raw(int fd, char *buffer, size_t buflen, unsigned
> int timeout)
>   {
>       // ...
>       len = read_smb_length_return_keepalive(fd,buffer,timeout);
>       // ...
>       if (len > buflen) {
>           DEBUG(0,("Invalid packet length! (%lu bytes).\n",(unsigned long)len));
>       //...
> 
> And here's the output (I added a debugging statement to also print buflen:
> 
> ##DRE: receive_smb_raw: Invalid packet length! len == (130107 bytes),
> buflen == (130048).
> 
> That is, it's complaining because 130107, the length reported in the
> reply packet header (I think), is larger than bufsize for the client
> state.  As to where the real problem is (i.e. does receive_smb_raw()
> need to do something with len before comparing it with buflen), I
> can't say.

Ah, I see the problem. CLI_SAMBA_MAX_LARGE_READX_SIZE is 127k
on the client, the server buffer size is 128k. For client large
readx/writex I should be allocating CLI_SAMBA_MAX_LARGE_READX_SIZE
+ LARGE_WRITEX_HDR_SIZE + SAFETY_MARGIN, not just
CLI_SAMBA_MAX_LARGE_READX_SIZE + SAFETY_MARGIN. It's safe as the
"safety margin" protects us but the client buffer detection
complains.

Try this patch against 3.0.x - should fix the problem.

Jeremy.
-------------- next part --------------
diff --git a/source/libsmb/cliconnect.c b/source/libsmb/cliconnect.c
index 3b97beb..632d910 100644
--- a/source/libsmb/cliconnect.c
+++ b/source/libsmb/cliconnect.c
@@ -1353,9 +1353,9 @@ bool cli_negprot(struct cli_state *cli)
 		if (cli->capabilities & (CAP_LARGE_READX|CAP_LARGE_WRITEX)) {
 			SAFE_FREE(cli->outbuf);
 			SAFE_FREE(cli->inbuf);
-			cli->outbuf = (char *)SMB_MALLOC(CLI_SAMBA_MAX_LARGE_READX_SIZE+SAFETY_MARGIN);
-			cli->inbuf = (char *)SMB_MALLOC(CLI_SAMBA_MAX_LARGE_READX_SIZE+SAFETY_MARGIN);
-			cli->bufsize = CLI_SAMBA_MAX_LARGE_READX_SIZE;
+			cli->outbuf = (char *)SMB_MALLOC(CLI_SAMBA_MAX_LARGE_READX_SIZE+LARGE_WRITEX_HDR_SIZE+SAFETY_MARGIN);
+			cli->inbuf = (char *)SMB_MALLOC(CLI_SAMBA_MAX_LARGE_READX_SIZE+LARGE_WRITEX_HDR_SIZE+SAFETY_MARGIN);
+			cli->bufsize = CLI_SAMBA_MAX_LARGE_READX_SIZE + LARGE_WRITEX_HDR_SIZE;
 		}
 
 	} else if (cli->protocol >= PROTOCOL_LANMAN1) {