Mercurial > dropbear
changeset 1062:210982935887 coverity
merge
author | Matt Johnston <matt@ucc.asn.au> |
---|---|
date | Mon, 02 Mar 2015 21:17:41 +0800 |
parents | e40d1b63b6a6 (current diff) a64ef3c2b371 (diff) |
children | 26c1f9b8f82e |
files | |
diffstat | 18 files changed, 183 insertions(+), 111 deletions(-) [+] |
line wrap: on
line diff
--- a/buffer.c Sat Feb 28 23:25:16 2015 +0800 +++ b/buffer.c Mon Mar 02 21:17:41 2015 +0800 @@ -46,17 +46,15 @@ dropbear_exit("buf->size too big"); } - buf = (buffer*)m_malloc(sizeof(buffer)); + buf = (buffer*)m_malloc(sizeof(buffer)+size); if (size > 0) { - buf->data = (unsigned char*)m_malloc(size); + buf->data = (unsigned char*)buf + sizeof(buffer); } else { buf->data = NULL; } buf->size = size; - buf->pos = 0; - buf->len = 0; return buf; @@ -65,7 +63,6 @@ /* free the buffer's data and the buffer itself */ void buf_free(buffer* buf) { - m_free(buf->data) m_free(buf); } @@ -78,17 +75,18 @@ /* resize a buffer, pos and len will be repositioned if required when * downsizing */ -void buf_resize(buffer *buf, unsigned int newsize) { +buffer* buf_resize(buffer *buf, unsigned int newsize) { if (newsize > BUF_MAX_SIZE) { dropbear_exit("buf->size too big"); } - buf->data = m_realloc(buf->data, newsize); + buf = m_realloc(buf, sizeof(buffer)+newsize); + buf->data = (unsigned char*)buf + sizeof(buffer); buf->size = newsize; buf->len = MIN(newsize, buf->len); buf->pos = MIN(newsize, buf->pos); - + return buf; } /* Create a copy of buf, allocating required memory etc. */ @@ -227,15 +225,15 @@ /* Return a string as a newly allocated buffer */ buffer * buf_getstringbuf(buffer *buf) { - buffer *ret; - unsigned char* str; - unsigned int len; - str = buf_getstring(buf, &len); - ret = m_malloc(sizeof(*ret)); - ret->data = str; - ret->len = len; - ret->size = len; - ret->pos = 0; + buffer *ret = NULL; + unsigned int len = buf_getint(buf); + if (len > MAX_STRING_LEN) { + dropbear_exit("String too long"); + } + ret = buf_new(len); + memcpy(buf_getwriteptr(ret, len), buf_getptr(buf, len), len); + buf_incrpos(buf, len); + buf_incrlen(ret, len); return ret; }
--- a/buffer.h Sat Feb 28 23:25:16 2015 +0800 +++ b/buffer.h Mon Mar 02 21:17:41 2015 +0800 @@ -29,7 +29,8 @@ #include "includes.h" struct buf { - + /* don't manipulate data member outside of buffer.c - it + is a pointer into the malloc holding buffer itself */ unsigned char * data; unsigned int len; /* the used size */ unsigned int pos; @@ -40,7 +41,8 @@ typedef struct buf buffer; buffer * buf_new(unsigned int size); -void buf_resize(buffer *buf, unsigned int newsize); +/* Possibly returns a new buffer*, like realloc() */ +buffer * buf_resize(buffer *buf, unsigned int newsize); void buf_free(buffer* buf); void buf_burn(buffer* buf); buffer* buf_newcopy(buffer* buf);
--- a/circbuffer.c Sat Feb 28 23:25:16 2015 +0800 +++ b/circbuffer.c Mon Mar 02 21:17:41 2015 +0800 @@ -110,6 +110,21 @@ return &cbuf->data[cbuf->readpos]; } +void cbuf_readptrs(circbuffer *cbuf, + unsigned char **p1, unsigned int *len1, + unsigned char **p2, unsigned int *len2) { + *p1 = &cbuf->data[cbuf->readpos]; + *len1 = MIN(cbuf->used, cbuf->size - cbuf->readpos); + + if (*len1 < cbuf->used) { + *p2 = cbuf->data; + *len2 = cbuf->used - *len1; + } else { + *p2 = NULL; + *len2 = 0; + } +} + unsigned char* cbuf_writeptr(circbuffer *cbuf, unsigned int len) { if (len > cbuf_writelen(cbuf)) { @@ -131,9 +146,11 @@ void cbuf_incrread(circbuffer *cbuf, unsigned int len) { +#if 0 if (len > cbuf_readlen(cbuf)) { dropbear_exit("Bad cbuf read"); } +#endif dropbear_assert(cbuf->used >= len); cbuf->used -= len;
--- a/circbuffer.h Sat Feb 28 23:25:16 2015 +0800 +++ b/circbuffer.h Mon Mar 02 21:17:41 2015 +0800 @@ -44,6 +44,9 @@ unsigned int cbuf_writelen(circbuffer *cbuf); /* max linear write len */ unsigned char* cbuf_readptr(circbuffer *cbuf, unsigned int len); +void cbuf_readptrs(circbuffer *cbuf, + unsigned char **p1, unsigned int *len1, + unsigned char **p2, unsigned int *len2); unsigned char* cbuf_writeptr(circbuffer *cbuf, unsigned int len); void cbuf_incrwrite(circbuffer *cbuf, unsigned int len); void cbuf_incrread(circbuffer *cbuf, unsigned int len);
--- a/cli-agentfwd.c Sat Feb 28 23:25:16 2015 +0800 +++ b/cli-agentfwd.c Mon Mar 02 21:17:41 2015 +0800 @@ -155,7 +155,7 @@ goto out; } - buf_resize(inbuf, readlen); + inbuf = buf_resize(inbuf, readlen); buf_setpos(inbuf, 0); ret = atomicio(read, fd, buf_getwriteptr(inbuf, readlen), readlen); if ((size_t)ret != readlen) {
--- a/common-channel.c Sat Feb 28 23:25:16 2015 +0800 +++ b/common-channel.c Mon Mar 02 21:17:41 2015 +0800 @@ -42,7 +42,8 @@ static void send_msg_channel_open_confirmation(struct Channel* channel, unsigned int recvwindow, unsigned int recvmaxpacket); -static void writechannel(struct Channel* channel, int fd, circbuffer *cbuf); +static void writechannel(struct Channel* channel, int fd, circbuffer *cbuf, + const unsigned char *moredata, unsigned int *morelen); static void send_msg_channel_window_adjust(struct Channel *channel, unsigned int incr); static void send_msg_channel_data(struct Channel *channel, int isextended); @@ -241,14 +242,14 @@ /* write to program/pipe stdin */ if (channel->writefd >= 0 && FD_ISSET(channel->writefd, writefds)) { - writechannel(channel, channel->writefd, channel->writebuf); + writechannel(channel, channel->writefd, channel->writebuf, NULL, NULL); do_check_close = 1; } /* stderr for client mode */ if (ERRFD_IS_WRITE(channel) && channel->errfd >= 0 && FD_ISSET(channel->errfd, writefds)) { - writechannel(channel, channel->errfd, channel->extrabuf); + writechannel(channel, channel->errfd, channel->extrabuf, NULL, NULL); do_check_close = 1; } @@ -434,14 +435,67 @@ } /* Called to write data out to the local side of the channel. - * Only called when we know we can write to a channel, writes as much as - * possible */ -static void writechannel(struct Channel* channel, int fd, circbuffer *cbuf) { + Writes the circular buffer contents and also the "moredata" buffer + if not null. Will ignore EAGAIN */ +static void writechannel(struct Channel* channel, int fd, circbuffer *cbuf, + const unsigned char *moredata, unsigned int *morelen) { - int len, maxlen; + struct iovec iov[3]; + unsigned char *circ_p1, *circ_p2; + unsigned int circ_len1, circ_len2; + int io_count = 0; + + int written; TRACE(("enter writechannel fd %d", fd)) + cbuf_readptrs(cbuf, &circ_p1, &circ_len1, &circ_p2, &circ_len2); + + if (circ_len1 > 0) { + TRACE(("circ1 %d", circ_len1)) + iov[io_count].iov_base = circ_p1; + iov[io_count].iov_len = circ_len1; + io_count++; + } + + if (circ_len2 > 0) { + TRACE(("circ2 %d", circ_len2)) + iov[io_count].iov_base = circ_p2; + iov[io_count].iov_len = circ_len2; + io_count++; + } + + if (morelen) { + assert(moredata); + TRACE(("more %d", *morelen)) + iov[io_count].iov_base = (void*)moredata; + iov[io_count].iov_len = *morelen; + io_count++; + } + + if (morelen) { + /* Default return value, none consumed */ + *morelen = 0; + } + + written = writev(fd, iov, io_count); + + if (written < 0) { + if (errno != EINTR && errno != EAGAIN) { + TRACE(("errno %d len %d", errno, len)) + close_chan_fd(channel, fd, SHUT_WR); + } + } else { + int cbuf_written = MIN(circ_len1+circ_len2, (unsigned int)written); + cbuf_incrread(cbuf, cbuf_written); + if (morelen) { + *morelen = written - cbuf_written; + } + channel->recvdonelen += written; + } + +#if 0 + maxlen = cbuf_readlen(cbuf); /* Write the data out */ @@ -458,10 +512,10 @@ cbuf_incrread(cbuf, len); channel->recvdonelen += len; +#endif /* Window adjust handling */ if (channel->recvdonelen >= RECV_WINDOWEXTEND) { - /* Set it back to max window */ send_msg_channel_window_adjust(channel, channel->recvdonelen); channel->recvwindow += channel->recvdonelen; channel->recvdonelen = 0; @@ -745,6 +799,7 @@ unsigned int maxdata; unsigned int buflen; unsigned int len; + unsigned int consumed; TRACE(("enter recv_msg_channel_data")) @@ -771,6 +826,17 @@ dropbear_exit("Oversized packet"); } + dropbear_assert(channel->recvwindow >= datalen); + channel->recvwindow -= datalen; + dropbear_assert(channel->recvwindow <= opts.recv_window); + + consumed = datalen; + writechannel(channel, fd, cbuf, buf_getptr(ses.payload, datalen), &consumed); + + datalen -= consumed; + buf_incrpos(ses.payload, consumed); + + /* We may have to run throught twice, if the buffer wraps around. Can't * just "leave it for next time" like with writechannel, since this * is payload data */ @@ -786,10 +852,6 @@ len -= buflen; } - dropbear_assert(channel->recvwindow >= datalen); - channel->recvwindow -= datalen; - dropbear_assert(channel->recvwindow <= opts.recv_window); - TRACE(("leave recv_msg_channel_data")) }
--- a/common-kex.c Sat Feb 28 23:25:16 2015 +0800 +++ b/common-kex.c Mon Mar 02 21:17:41 2015 +0800 @@ -534,8 +534,10 @@ buf_putstring(ses.kexhashbuf, ses.transkexinit->data, ses.transkexinit->len); /* I_S, the payload of the server's SSH_MSG_KEXINIT */ - buf_setpos(ses.payload, 0); - buf_putstring(ses.kexhashbuf, ses.payload->data, ses.payload->len); + buf_setpos(ses.payload, ses.payload_beginning); + buf_putstring(ses.kexhashbuf, + buf_getptr(ses.payload, ses.payload->len-ses.payload->pos), + ses.payload->len-ses.payload->pos); ses.requirenext = SSH_MSG_KEXDH_REPLY; } else { /* SERVER */ @@ -549,8 +551,10 @@ (unsigned char*)LOCAL_IDENT, local_ident_len); /* I_C, the payload of the client's SSH_MSG_KEXINIT */ - buf_setpos(ses.payload, 0); - buf_putstring(ses.kexhashbuf, ses.payload->data, ses.payload->len); + buf_setpos(ses.payload, ses.payload_beginning); + buf_putstring(ses.kexhashbuf, + buf_getptr(ses.payload, ses.payload->len-ses.payload->pos), + ses.payload->len-ses.payload->pos); /* I_S, the payload of the server's SSH_MSG_KEXINIT */ buf_putstring(ses.kexhashbuf,
--- a/common-session.c Sat Feb 28 23:25:16 2015 +0800 +++ b/common-session.c Mon Mar 02 21:17:41 2015 +0800 @@ -152,8 +152,10 @@ FD_ZERO(&readfd); dropbear_assert(ses.payload == NULL); - /* during initial setup we flush out the KEXINIT packet before - * attempting to read the remote version string, which might block */ + /* We delay reading from the input socket during initial setup until + after we have written out our initial KEXINIT packet (empty writequeue). + This means our initial packet can be in-flight while we're doing a blocking + read for the remote ident */ if (ses.sock_in != -1 && (ses.remoteident || isempty(&ses.writequeue))) { FD_SET(ses.sock_in, &readfd); }
--- a/ecc.c Sat Feb 28 23:25:16 2015 +0800 +++ b/ecc.c Mon Mar 02 21:17:41 2015 +0800 @@ -86,11 +86,6 @@ { mp_int *prime, *b, *t1, *t2; int err; - - prime = m_malloc(sizeof(mp_int)); - b = m_malloc(sizeof(mp_int)); - t1 = m_malloc(sizeof(mp_int)); - t2 = m_malloc(sizeof(mp_int)); m_mp_alloc_init_multi(&prime, &b, &t1, &t2, NULL);
--- a/ecdsa.c Sat Feb 28 23:25:16 2015 +0800 +++ b/ecdsa.c Mon Mar 02 21:17:41 2015 +0800 @@ -409,7 +409,7 @@ out: ltc_ecc_del_point(mG); ltc_ecc_del_point(mQ); - mp_clear_multi(r, s, v, w, u1, u2, p, e, m, NULL); + ltc_deinit_multi(r, s, v, w, u1, u2, p, e, m, NULL); if (mp != NULL) { ltc_mp.montgomery_deinit(mp); }
--- a/libtomcrypt/src/mac/hmac/hmac_done.c Sat Feb 28 23:25:16 2015 +0800 +++ b/libtomcrypt/src/mac/hmac/hmac_done.c Mon Mar 02 21:17:41 2015 +0800 @@ -28,7 +28,7 @@ */ int hmac_done(hmac_state *hmac, unsigned char *out, unsigned long *outlen) { - unsigned char *buf, *isha; + unsigned char buf[MAXBLOCKSIZE], isha[MAXBLOCKSIZE]; unsigned long hashsize, i; int hash, err; @@ -44,19 +44,6 @@ /* get the hash message digest size */ hashsize = hash_descriptor[hash].hashsize; - /* allocate buffers */ - buf = XMALLOC(HMAC_BLOCKSIZE); - isha = XMALLOC(hashsize); - if (buf == NULL || isha == NULL) { - if (buf != NULL) { - XFREE(buf); - } - if (isha != NULL) { - XFREE(isha); - } - return CRYPT_MEM; - } - /* Get the hash of the first HMAC vector plus the data */ if ((err = hash_descriptor[hash].done(&hmac->md, isha)) != CRYPT_OK) { goto LBL_ERR; @@ -96,9 +83,6 @@ zeromem(hmac, sizeof(*hmac)); #endif - XFREE(isha); - XFREE(buf); - return err; }
--- a/libtomcrypt/src/mac/hmac/hmac_init.c Sat Feb 28 23:25:16 2015 +0800 +++ b/libtomcrypt/src/mac/hmac/hmac_init.c Mon Mar 02 21:17:41 2015 +0800 @@ -29,7 +29,7 @@ */ int hmac_init(hmac_state *hmac, int hash, const unsigned char *key, unsigned long keylen) { - unsigned char *buf; + unsigned char buf[MAXBLOCKSIZE]; unsigned long hashsize; unsigned long i, z; int err; @@ -49,16 +49,9 @@ return CRYPT_INVALID_KEYSIZE; } - /* allocate ram for buf */ - buf = XMALLOC(HMAC_BLOCKSIZE); - if (buf == NULL) { - return CRYPT_MEM; - } - /* allocate memory for key */ hmac->key = XMALLOC(HMAC_BLOCKSIZE); if (hmac->key == NULL) { - XFREE(buf); return CRYPT_MEM; } @@ -101,7 +94,6 @@ zeromem(buf, HMAC_BLOCKSIZE); #endif - XFREE(buf); return err; }
--- a/netio.c Sat Feb 28 23:25:16 2015 +0800 +++ b/netio.c Mon Mar 02 21:17:41 2015 +0800 @@ -169,11 +169,10 @@ snprintf(c->errstring, len, "Error resolving '%s' port '%s'. %s", remotehost, remoteport, gai_strerror(err)); TRACE(("Error resolving: %s", gai_strerror(err))) - return NULL; + } else { + c->res_iter = c->res; } - c->res_iter = c->res; - return c; } @@ -220,7 +219,7 @@ socklen_t vallen = sizeof(val); struct dropbear_progress_connection *c = iter->item; - if (!FD_ISSET(c->sock, writefd)) { + if (c->sock < 0 || !FD_ISSET(c->sock, writefd)) { continue; }
--- a/netio.h Sat Feb 28 23:25:16 2015 +0800 +++ b/netio.h Mon Mar 02 21:17:41 2015 +0800 @@ -27,6 +27,7 @@ errstring is only set on DROPBEAR_FAILURE, returns failure message for the last attempted socket */ typedef void(*connect_callback)(int result, int sock, void* data, const char* errstring); +/* Always returns a progress connection, if it fails it will call the callback at a later point */ struct dropbear_progress_connection * connect_remote (const char* remotehost, const char* remoteport, connect_callback cb, void *cb_data);
--- a/packet.c Sat Feb 28 23:25:16 2015 +0800 +++ b/packet.c Mon Mar 02 21:17:41 2015 +0800 @@ -257,7 +257,7 @@ } if (len > ses.readbuf->size) { - buf_resize(ses.readbuf, len); + ses.readbuf = buf_resize(ses.readbuf, len); } buf_setlen(ses.readbuf, len); buf_setpos(ses.readbuf, blocksize); @@ -314,18 +314,21 @@ if (is_compress_recv()) { /* decompress */ ses.payload = buf_decompress(ses.readbuf, len); + buf_setpos(ses.payload, 0); + ses.payload_beginning = 0; + buf_free(ses.readbuf); } else #endif { + ses.payload = ses.readbuf; + ses.payload_beginning = ses.payload->pos; + buf_setlen(ses.payload, ses.payload->pos + len); /* copy payload */ - ses.payload = buf_new(len); - memcpy(ses.payload->data, buf_getptr(ses.readbuf, len), len); - buf_incrlen(ses.payload, len); + //ses.payload = buf_new(len); + //memcpy(ses.payload->data, buf_getptr(ses.readbuf, len), len); + //buf_incrlen(ses.payload, len); } - - buf_free(ses.readbuf); ses.readbuf = NULL; - buf_setpos(ses.payload, 0); ses.recvseq++; @@ -398,7 +401,7 @@ dropbear_exit("bad packet, oversized decompressed"); } new_size = MIN(RECV_MAX_PAYLOAD_LEN, ret->size + ZLIB_DECOMPRESS_INCR); - buf_resize(ret, new_size); + ret = buf_resize(ret, new_size); } } } @@ -637,7 +640,8 @@ #ifndef DISABLE_ZLIB /* compresses len bytes from src, outputting to dest (starting from the - * respective current positions. */ + * respective current positions. dest must have sufficient space, + * len+ZLIB_COMPRESS_EXPANSION */ static void buf_compress(buffer * dest, buffer * src, unsigned int len) { unsigned int endpos = src->pos + len; @@ -645,38 +649,28 @@ TRACE2(("enter buf_compress")) - while (1) { - - ses.keys->trans.zstream->avail_in = endpos - src->pos; - ses.keys->trans.zstream->next_in = - buf_getptr(src, ses.keys->trans.zstream->avail_in); + dropbear_assert(dest->size - dest->pos >= len+ZLIB_COMPRESS_EXPANSION); - ses.keys->trans.zstream->avail_out = dest->size - dest->pos; - ses.keys->trans.zstream->next_out = - buf_getwriteptr(dest, ses.keys->trans.zstream->avail_out); + ses.keys->trans.zstream->avail_in = endpos - src->pos; + ses.keys->trans.zstream->next_in = + buf_getptr(src, ses.keys->trans.zstream->avail_in); - result = deflate(ses.keys->trans.zstream, Z_SYNC_FLUSH); - - buf_setpos(src, endpos - ses.keys->trans.zstream->avail_in); - buf_setlen(dest, dest->size - ses.keys->trans.zstream->avail_out); - buf_setpos(dest, dest->len); + ses.keys->trans.zstream->avail_out = dest->size - dest->pos; + ses.keys->trans.zstream->next_out = + buf_getwriteptr(dest, ses.keys->trans.zstream->avail_out); - if (result != Z_OK) { - dropbear_exit("zlib error"); - } + result = deflate(ses.keys->trans.zstream, Z_SYNC_FLUSH); - if (ses.keys->trans.zstream->avail_in == 0) { - break; - } + buf_setpos(src, endpos - ses.keys->trans.zstream->avail_in); + buf_setlen(dest, dest->size - ses.keys->trans.zstream->avail_out); + buf_setpos(dest, dest->len); - dropbear_assert(ses.keys->trans.zstream->avail_out == 0); + if (result != Z_OK) { + dropbear_exit("zlib error"); + } - /* the buffer has been filled, we must extend. This only happens in - * unusual circumstances where the data grows in size after deflate(), - * but it is possible */ - buf_resize(dest, dest->size + ZLIB_COMPRESS_EXPANSION); - - } + /* fails if destination buffer wasn't large enough */ + dropbear_assert(ses.keys->trans.zstream->avail_in == 0); TRACE2(("leave buf_compress")) } #endif
--- a/session.h Sat Feb 28 23:25:16 2015 +0800 +++ b/session.h Mon Mar 02 21:17:41 2015 +0800 @@ -126,7 +126,11 @@ buffer with the packet to send. */ struct Queue writequeue; /* A queue of encrypted packets to send */ buffer *readbuf; /* From the wire, decrypted in-place */ - buffer *payload; /* Post-decompression, the actual SSH packet */ + buffer *payload; /* Post-decompression, the actual SSH packet. + May have extra data at the beginning, will be + passed to packet processing functions positioned past + that, see payload_beginning */ + unsigned int payload_beginning; unsigned int transseq, recvseq; /* Sequence IDs */ /* Packet-handling flags */
--- a/signkey.c Sat Feb 28 23:25:16 2015 +0800 +++ b/signkey.c Mon Mar 02 21:17:41 2015 +0800 @@ -187,6 +187,7 @@ if (eck) { if (*eck) { ecc_free(*eck); + m_free(*eck); *eck = NULL; } *eck = buf_get_ecdsa_pub_key(buf); @@ -255,6 +256,7 @@ if (eck) { if (*eck) { ecc_free(*eck); + m_free(*eck); *eck = NULL; } *eck = buf_get_ecdsa_priv_key(buf); @@ -355,18 +357,21 @@ #ifdef DROPBEAR_ECC_256 if (key->ecckey256) { ecc_free(key->ecckey256); + m_free(key->ecckey256); key->ecckey256 = NULL; } #endif #ifdef DROPBEAR_ECC_384 if (key->ecckey384) { ecc_free(key->ecckey384); + m_free(key->ecckey384); key->ecckey384 = NULL; } #endif #ifdef DROPBEAR_ECC_521 if (key->ecckey521) { ecc_free(key->ecckey521); + m_free(key->ecckey521); key->ecckey521 = NULL; } #endif
--- a/svr-authpubkey.c Sat Feb 28 23:25:16 2015 +0800 +++ b/svr-authpubkey.c Mon Mar 02 21:17:41 2015 +0800 @@ -86,6 +86,7 @@ unsigned int algolen; unsigned char* keyblob = NULL; unsigned int keybloblen; + unsigned int sign_payload_length; buffer * signbuf = NULL; sign_key * key = NULL; char* fp = NULL; @@ -125,9 +126,18 @@ /* create the data which has been signed - this a string containing * session_id, concatenated with the payload packet up to the signature */ + assert(ses.payload_beginning <= ses.payload->pos); + sign_payload_length = ses.payload->pos - ses.payload_beginning; signbuf = buf_new(ses.payload->pos + 4 + ses.session_id->len); buf_putbufstring(signbuf, ses.session_id); - buf_putbytes(signbuf, ses.payload->data, ses.payload->pos); + + /* The entire contents of the payload prior. */ + buf_setpos(ses.payload, ses.payload_beginning); + buf_putbytes(signbuf, + buf_getptr(ses.payload, sign_payload_length), + sign_payload_length); + buf_incrpos(ses.payload, sign_payload_length); + buf_setpos(signbuf, 0); /* ... and finally verify the signature */