Security fix for CVE-2008-1105: Boundary failure when parsing SMB responses
authorJeremy Allison <jra@samba.org>
Wed, 28 May 2008 16:31:42 +0000 (09:31 -0700)
committerJeremy Allison <jra@samba.org>
Wed, 28 May 2008 16:31:42 +0000 (09:31 -0700)
can result in a buffer overrun.
Jeremy.
(This used to be commit 23b825e9d2c74c5b940cf4d3aa56c18692259972)

source3/client/client.c
source3/include/proto.h
source3/lib/util_sock.c
source3/libsmb/clientgen.c
source3/libsmb/clireadwrite.c
source3/smbd/process.c
source3/utils/smbfilter.c

index cc0da18d4d36c813a390e2b856307f4eb76ad09c..8c939fc3ec6b555f0dbfc35ef026c692378c217d 100644 (file)
@@ -4382,7 +4382,7 @@ static void readline_callback(void)
 
                set_smb_read_error(&cli->smb_rw_error, SMB_READ_OK);
 
-               status = receive_smb_raw(cli->fd, cli->inbuf, 0, 0, &len);
+               status = receive_smb_raw(cli->fd, cli->inbuf, cli->bufsize, 0, 0, &len);
 
                if (!NT_STATUS_IS_OK(status)) {
                        DEBUG(0, ("Read from server failed, maybe it closed "
index 36d75a4d75782fbe6942c9fff95ef7139b43637b..761c72049753f13ad0bb7f92b0020b5253944044 100644 (file)
@@ -1582,8 +1582,12 @@ NTSTATUS read_smb_length_return_keepalive(int fd, char *inbuf,
                                          size_t *len);
 NTSTATUS read_smb_length(int fd, char *inbuf, unsigned int timeout,
                         size_t *len);
-NTSTATUS receive_smb_raw(int fd, char *buffer, unsigned int timeout,
-                        size_t maxlen, size_t *p_len);
+NTSTATUS receive_smb_raw(int fd,
+                       char *buffer,
+                       size_t buflen,
+                       unsigned int timeout,
+                       size_t maxlen,
+                       size_t *p_len);
 int open_socket_in(int type,
                uint16_t port,
                int dlevel,
index f252377b7ed88a03318c75c48e4c6c075a52f8bd..b2a1ece5db7fc2ddf411ef8ba72f200215010502 100644 (file)
@@ -1151,16 +1151,15 @@ NTSTATUS read_smb_length(int fd, char *inbuf, unsigned int timeout,
 }
 
 /****************************************************************************
- Read an smb from a fd. Note that the buffer *MUST* be of size
- BUFFER_SIZE+SAFETY_MARGIN.
+ Read an smb from a fd.
  The timeout is in milliseconds.
  This function will return on receipt of a session keepalive packet.
  maxlen is the max number of bytes to return, not including the 4 byte
- length. If zero it means BUFFER_SIZE+SAFETY_MARGIN limit.
+ length. If zero it means buflen limit.
  Doesn't check the MAC on signed packets.
 ****************************************************************************/
 
-NTSTATUS receive_smb_raw(int fd, char *buffer, unsigned int timeout,
+NTSTATUS receive_smb_raw(int fd, char *buffer, size_t buflen, unsigned int timeout,
                         size_t maxlen, size_t *p_len)
 {
        size_t len;
@@ -1173,17 +1172,10 @@ NTSTATUS receive_smb_raw(int fd, char *buffer, unsigned int timeout,
                return status;
        }
 
-       /*
-        * A WRITEX with CAP_LARGE_WRITEX can be 64k worth of data plus 65 bytes
-        * of header. Don't print the error if this fits.... JRA.
-        */
-
-       if (len > (BUFFER_SIZE + LARGE_WRITEX_HDR_SIZE)) {
+       if (len > buflen) {
                DEBUG(0,("Invalid packet length! (%lu bytes).\n",
                                        (unsigned long)len));
-               if (len > BUFFER_SIZE + (SAFETY_MARGIN/2)) {
-                       return NT_STATUS_INVALID_PARAMETER;
-               }
+               return NT_STATUS_INVALID_PARAMETER;
        }
 
        if(len > 0) {
index e64b6fa278a233f08ac24102c5ff23e33e90ff9b..60ec632b838939b5d103f89ced750a438155754a 100644 (file)
@@ -57,8 +57,7 @@ int cli_set_port(struct cli_state *cli, int port)
 }
 
 /****************************************************************************
- Read an smb from a fd ignoring all keepalive packets. Note that the buffer 
- *MUST* be of size BUFFER_SIZE+SAFETY_MARGIN.
+ Read an smb from a fd ignoring all keepalive packets.
  The timeout is in milliseconds
 
  This is exactly the same as receive_smb except that it never returns
@@ -76,8 +75,8 @@ static ssize_t client_receive_smb(struct cli_state *cli, size_t maxlen)
 
                set_smb_read_error(&cli->smb_rw_error, SMB_READ_OK);
 
-               status = receive_smb_raw(cli->fd, cli->inbuf, cli->timeout,
-                                        maxlen, &len);
+               status = receive_smb_raw(cli->fd, cli->inbuf, cli->bufsize,
+                                       cli->timeout, maxlen, &len);
                if (!NT_STATUS_IS_OK(status)) {
                        DEBUG(10,("client_receive_smb failed\n"));
                        show_msg(cli->inbuf);
@@ -225,93 +224,6 @@ ssize_t cli_receive_smb_data(struct cli_state *cli, char *buffer, size_t len)
        return -1;
 }
 
-/****************************************************************************
- Read a smb readX header.
- We can only use this if encryption and signing are off.
-****************************************************************************/
-
-bool cli_receive_smb_readX_header(struct cli_state *cli)
-{
-       ssize_t len, offset;
-
-       if (cli->fd == -1)
-               return false; 
-
- again:
-
-       /* Read up to the size of a readX header reply. */
-       len = client_receive_smb(cli, (smb_size - 4) + 24);
-       
-       if (len > 0) {
-               /* it might be an oplock break request */
-               if (!(CVAL(cli->inbuf, smb_flg) & FLAG_REPLY) &&
-                   CVAL(cli->inbuf,smb_com) == SMBlockingX &&
-                   SVAL(cli->inbuf,smb_vwv6) == 0 &&
-                   SVAL(cli->inbuf,smb_vwv7) == 0) {
-                       ssize_t total_len = smb_len(cli->inbuf);
-
-                       if (total_len > CLI_SAMBA_MAX_LARGE_READX_SIZE+SAFETY_MARGIN) {
-                               goto read_err;
-                       }
-
-                       /* Read the rest of the data. */
-                       if ((total_len - len > 0) &&
-                           !cli_receive_smb_data(cli,cli->inbuf+len,total_len - len)) {
-                               goto read_err;
-                       }
-
-                       if (cli->oplock_handler) {
-                               int fnum = SVAL(cli->inbuf,smb_vwv2);
-                               unsigned char level = CVAL(cli->inbuf,smb_vwv3+1);
-                               if (!cli->oplock_handler(cli, fnum, level)) return false;
-                       }
-                       /* try to prevent loops */
-                       SCVAL(cli->inbuf,smb_com,0xFF);
-                       goto again;
-               }
-       }
-
-       /* If it's not the above size it probably was an error packet. */
-
-       if ((len == (smb_size - 4) + 24) && !cli_is_error(cli)) {
-               /* Check it's a non-chained readX reply. */
-               if (!(CVAL(cli->inbuf, smb_flg) & FLAG_REPLY) ||
-                       (CVAL(cli->inbuf,smb_vwv0) != 0xFF) ||
-                       (CVAL(cli->inbuf,smb_com) != SMBreadX)) {
-                       /* 
-                        * We're not coping here with asnyc replies to
-                        * other calls. Punt here - we need async client
-                        * libs for this.
-                        */
-                       goto read_err;
-               }
-
-               /* 
-                * We know it's a readX reply - ensure we've read the
-                * padding bytes also.
-                */
-
-               offset = SVAL(cli->inbuf,smb_vwv6);
-               if (offset > len) {
-                       ssize_t ret;
-                       size_t padbytes = offset - len;
-                       ret = cli_receive_smb_data(cli,smb_buf(cli->inbuf),padbytes);
-                       if (ret != padbytes) {
-                               goto read_err;
-                       }
-               }
-       }
-
-       return true;
-
-  read_err:
-
-       cli->smb_rw_error = SMB_READ_ERROR;
-       close(cli->fd);
-       cli->fd = -1;
-       return false;
-}
-
 static ssize_t write_socket(int fd, const char *buf, size_t len)
 {
         ssize_t ret=0;
index 515471e0030ddaa3f660133714ec61d9859da50c..057e647983dea5a71e72dda45b8a940b680d489c 100644 (file)
@@ -472,106 +472,6 @@ ssize_t cli_read(struct cli_state *cli, int fnum, char *buf,
        return ret;
 }
 
-#if 0  /* relies on client_receive_smb(), now a static in libsmb/clientgen.c */
-
-/* This call is INCOMPATIBLE with SMB signing.  If you remove the #if 0
-   you must fix ensure you don't attempt to sign the packets - data
-   *will* be currupted */
-
-/****************************************************************************
-Issue a single SMBreadraw and don't wait for a reply.
-****************************************************************************/
-
-static bool cli_issue_readraw(struct cli_state *cli, int fnum, off_t offset, 
-                          size_t size, int i)
-{
-
-       if (!cli->sign_info.use_smb_signing) {
-               DEBUG(0, ("Cannot use readraw and SMB Signing\n"));
-               return False;
-       }
-       
-       memset(cli->outbuf,'\0',smb_size);
-       memset(cli->inbuf,'\0',smb_size);
-
-       cli_set_message(cli->outbuf,10,0,True);
-               
-       SCVAL(cli->outbuf,smb_com,SMBreadbraw);
-       SSVAL(cli->outbuf,smb_tid,cli->cnum);
-       cli_setup_packet(cli);
-
-       SSVAL(cli->outbuf,smb_vwv0,fnum);
-       SIVAL(cli->outbuf,smb_vwv1,offset);
-       SSVAL(cli->outbuf,smb_vwv2,size);
-       SSVAL(cli->outbuf,smb_vwv3,size);
-       SSVAL(cli->outbuf,smb_mid,cli->mid + i);
-
-       return cli_send_smb(cli);
-}
-
-/****************************************************************************
- Tester for the readraw call.
-****************************************************************************/
-
-ssize_t cli_readraw(struct cli_state *cli, int fnum, char *buf, off_t offset, size_t size)
-{
-       char *p;
-       int size2;
-       size_t readsize;
-       ssize_t total = 0;
-
-       if (size == 0) 
-               return 0;
-
-       /*
-        * Set readsize to the maximum size we can handle in one readraw.
-        */
-
-       readsize = 0xFFFF;
-
-       while (total < size) {
-               readsize = MIN(readsize, size-total);
-
-               /* Issue a read and receive a reply */
-
-               if (!cli_issue_readraw(cli, fnum, offset, readsize, 0))
-                       return -1;
-
-               if (!client_receive_smb(cli->fd, cli->inbuf, cli->timeout))
-                       return -1;
-
-               size2 = smb_len(cli->inbuf);
-
-               if (size2 > readsize) {
-                       DEBUG(5,("server returned more than we wanted!\n"));
-                       return -1;
-               } else if (size2 < 0) {
-                       DEBUG(5,("read return < 0!\n"));
-                       return -1;
-               }
-
-               /* Copy data into buffer */
-
-               if (size2) {
-                       p = cli->inbuf + 4;
-                       memcpy(buf + total, p, size2);
-               }
-
-               total += size2;
-               offset += size2;
-
-               /*
-                * If the server returned less than we asked for we're at EOF.
-                */
-
-               if (size2 < readsize)
-                       break;
-       }
-
-       return total;
-}
-#endif
-
 /****************************************************************************
  Issue a single SMBwrite and don't wait for a reply.
 ****************************************************************************/
index c8ad19dd15ca797a243564dca30a6b73183f823d..71e38634b70ace96f0212d74f9bc56b242d86c9b 100644 (file)
@@ -120,9 +120,7 @@ static bool valid_packet_size(size_t len)
        if (len > (BUFFER_SIZE + LARGE_WRITEX_HDR_SIZE)) {
                DEBUG(0,("Invalid packet length! (%lu bytes).\n",
                                        (unsigned long)len));
-               if (len > BUFFER_SIZE + (SAFETY_MARGIN/2)) {
-                       return false;
-               }
+               return false;
        }
        return true;
 }
index e128e1ce345c2c1dc55c301eb4e70a5f9829324d..d274e0929909b6fdec2e677514dddaf0da31cc16 100644 (file)
@@ -171,7 +171,8 @@ static void filter_child(int c, struct sockaddr_storage *dest_ss)
                if (c != -1 && FD_ISSET(c, &fds)) {
                        size_t len;
                        if (!NT_STATUS_IS_OK(receive_smb_raw(
-                                                    c, packet, 0, 0, &len))) {
+                                                       c, packet, sizeof(packet),
+                                                       0, 0, &len))) {
                                d_printf("client closed connection\n");
                                exit(0);
                        }
@@ -184,7 +185,8 @@ static void filter_child(int c, struct sockaddr_storage *dest_ss)
                if (s != -1 && FD_ISSET(s, &fds)) {
                        size_t len;
                        if (!NT_STATUS_IS_OK(receive_smb_raw(
-                                                    s, packet, 0, 0, &len))) {
+                                                       s, packet, sizeof(packet),
+                                                       0, 0, &len))) {
                                d_printf("server closed connection\n");
                                exit(0);
                        }