From d36434f31268b75040311352f23c92c9a61e8cda Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 28 May 2008 09:31:42 -0700 Subject: Security fix for CVE-2008-1105: Boundary failure when parsing SMB responses can result in a buffer overrun. Jeremy. (This used to be commit 23b825e9d2c74c5b940cf4d3aa56c18692259972) --- source3/client/client.c | 2 +- source3/include/proto.h | 8 +++- source3/lib/util_sock.c | 18 +++----- source3/libsmb/clientgen.c | 94 ++------------------------------------- source3/libsmb/clireadwrite.c | 100 ------------------------------------------ source3/smbd/process.c | 4 +- source3/utils/smbfilter.c | 6 ++- 7 files changed, 20 insertions(+), 212 deletions(-) (limited to 'source3') diff --git a/source3/client/client.c b/source3/client/client.c index cc0da18d4d..8c939fc3ec 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -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 " diff --git a/source3/include/proto.h b/source3/include/proto.h index 36d75a4d75..761c720497 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -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, diff --git a/source3/lib/util_sock.c b/source3/lib/util_sock.c index f252377b7e..b2a1ece5db 100644 --- a/source3/lib/util_sock.c +++ b/source3/lib/util_sock.c @@ -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) { diff --git a/source3/libsmb/clientgen.c b/source3/libsmb/clientgen.c index e64b6fa278..60ec632b83 100644 --- a/source3/libsmb/clientgen.c +++ b/source3/libsmb/clientgen.c @@ -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; diff --git a/source3/libsmb/clireadwrite.c b/source3/libsmb/clireadwrite.c index 515471e003..057e647983 100644 --- a/source3/libsmb/clireadwrite.c +++ b/source3/libsmb/clireadwrite.c @@ -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. ****************************************************************************/ diff --git a/source3/smbd/process.c b/source3/smbd/process.c index c8ad19dd15..71e38634b7 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -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; } diff --git a/source3/utils/smbfilter.c b/source3/utils/smbfilter.c index e128e1ce34..d274e09299 100644 --- a/source3/utils/smbfilter.c +++ b/source3/utils/smbfilter.c @@ -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); } -- cgit