ref: d0c82543d55f1f5938460022777301b8de023bc9
parent: cc69f7efa898d0ffd547bddbabf50881a790fbe5
author: Timothy B. Terriberry <tterribe@xiph.org>
date: Fri Sep 16 11:41:27 EDT 2016
Fix MSVC warnings. Some of these pointed to real potential overflows (given arbitrary inputs by the calling application). I was sad about stripping const qualifiers from the struct addrinfo pointers, but MSVC seems to erroneously think that an array of pointers to constant data is itself a pointer to constant data (or maybe that it is not compatible with a const void *?), and converting the memmove()s to for loops triggered an erroneous warning about out-of-bounds array accesses in gcc (but on only one of the two identical loops).
--- a/src/http.c
+++ b/src/http.c
@@ -307,6 +307,12 @@
operate on sockets, because we don't use non-socket I/O here, and this
minimizes the changes needed to deal with Winsock.*/
# define close(_fd) closesocket(_fd)
+/*This takes an int for the address length, even though the value is of type
+ socklen_t (defined as an unsigned integer type with at least 32 bits).*/
+# define connect(_fd,_addr,_addrlen) \
+ (OP_UNLIKELY((_addrlen)>(socklen_t)INT_MAX)? \
+ WSASetLastError(WSA_NOT_ENOUGH_MEMORY),-1: \
+ connect(_fd,_addr,(int)(_addrlen)))
/*This relies on sizeof(u_long)==sizeof(int), which is always true on both
Win32 and Win64.*/
# define ioctl(_fd,_req,_arg) ioctlsocket(_fd,_req,(u_long *)(_arg))
@@ -471,7 +477,7 @@
scheme_end=_src+strspn(_src,OP_URL_SCHEME);
if(OP_UNLIKELY(*scheme_end!=':')
||OP_UNLIKELY(scheme_end-_src<4)||OP_UNLIKELY(scheme_end-_src>5)
- ||OP_UNLIKELY(op_strncasecmp(_src,"https",scheme_end-_src)!=0)){+ ||OP_UNLIKELY(op_strncasecmp(_src,"https",(int)(scheme_end-_src))!=0)){/*Unsupported protocol.*/
return OP_EIMPL;
}
@@ -674,7 +680,10 @@
}
static int op_sb_append_string(OpusStringBuf *_sb,const char *_s){- return op_sb_append(_sb,_s,strlen(_s));
+ size_t len;
+ len=strlen(_s);
+ if(OP_UNLIKELY(len>(size_t)INT_MAX))return OP_EFAULT;
+ return op_sb_append(_sb,_s,(int)len);
}
static int op_sb_append_port(OpusStringBuf *_sb,unsigned _port){@@ -962,7 +971,8 @@
ret=send(fd.fd,_buf,_buf_size,0);
if(ret>0){_buf+=ret;
- _buf_size-=ret;
+ OP_ASSERT(ret<=_buf_size);
+ _buf_size-=(int)ret;
continue;
}
err=op_errno();
@@ -1077,8 +1087,9 @@
if(ret>0){/*Read some data.
Keep going to see if there's more.*/
- nread+=ret;
- nread_unblocked+=ret;
+ OP_ASSERT(ret<=_buf_size-nread);
+ nread+=(int)ret;
+ nread_unblocked+=(int)ret;
continue;
}
/*If we already read some data or the connection was closed, return
@@ -1620,6 +1631,7 @@
size_t pattern_label_len;
size_t pattern_prefix_len;
size_t pattern_suffix_len;
+ if(OP_UNLIKELY(_host_len>(size_t)INT_MAX))return 0;
pattern=(const char *)ASN1_STRING_data(_pattern);
pattern_len=strlen(pattern);
/*Check the pattern for embedded NULs.*/
@@ -1627,6 +1639,7 @@
pattern_label_len=strcspn(pattern,".");
OP_ASSERT(pattern_label_len<=pattern_len);
pattern_prefix_len=strcspn(pattern,"*");
+ if(OP_UNLIKELY(pattern_prefix_len>(size_t)INT_MAX))return 0;
if(pattern_prefix_len>=pattern_label_len){/*"The client SHOULD NOT attempt to match a presented identifier in which
the wildcard character comprises a label other than the left-most label
@@ -1637,7 +1650,8 @@
Don't use the system strcasecmp here, as that uses the locale and
RFC 4343 makes clear that DNS's case-insensitivity only applies to
the ASCII range.*/
- return _host_len==pattern_len&&op_strncasecmp(_host,pattern,_host_len)==0;
+ return _host_len==pattern_len
+ &&op_strncasecmp(_host,pattern,(int)_host_len)==0;
}
/*"However, the client SHOULD NOT attempt to match a presented identifier
where the wildcard character is embedded within an A-label or U-label of
@@ -1672,10 +1686,11 @@
pattern_suffix_len=pattern_len-pattern_prefix_len-1;
host_suffix_len=_host_len-host_label_len
+pattern_label_len-pattern_prefix_len-1;
+ OP_ASSERT(host_suffix_len<=_host_len);
return pattern_suffix_len==host_suffix_len
- &&op_strncasecmp(_host,pattern,pattern_prefix_len)==0
+ &&op_strncasecmp(_host,pattern,(int)pattern_prefix_len)==0
&&op_strncasecmp(_host+_host_len-host_suffix_len,
- pattern+pattern_prefix_len+1,host_suffix_len)==0;
+ pattern+pattern_prefix_len+1,(int)host_suffix_len)==0;
}
/*Convert a host to a numeric address, if possible.
@@ -1851,7 +1866,8 @@
BIO *ssl_bio;
int skip_certificate_check;
int ret;
- ssl_bio=BIO_new_socket(_fd,BIO_NOCLOSE);
+ /*This always takes an int, even though with Winsock op_sock is a SOCKET.*/
+ ssl_bio=BIO_new_socket((int)_fd,BIO_NOCLOSE);
if(OP_LIKELY(ssl_bio==NULL))return OP_FALSE;
# if !defined(OPENSSL_NO_TLSEXT)
/*Support for RFC 6066 Server Name Indication.*/
@@ -1911,11 +1927,10 @@
left to try.
*_addr will be set to NULL in this case.*/
static int op_sock_connect_next(op_sock _fd,
- const struct addrinfo **_addr,int _ai_family){- const struct addrinfo *addr;
- int err;
- addr=*_addr;
- for(;;){+ struct addrinfo **_addr,int _ai_family){+ struct addrinfo *addr;
+ int err;
+ for(addr=*_addr;;addr=addr->ai_next){/*Move to the next address of the requested type.*/
for(;addr!=NULL&&addr->ai_family!=_ai_family;addr=addr->ai_next);
*_addr=addr;
@@ -1925,7 +1940,6 @@
err=op_errno();
/*Winsock will set WSAEWOULDBLOCK.*/
if(OP_LIKELY(err==EINPROGRESS||err==EWOULDBLOCK))return 0;
- addr=addr->ai_next;
}
}
@@ -1933,15 +1947,15 @@
# define OP_NPROTOS (2)
static int op_http_connect_impl(OpusHTTPStream *_stream,OpusHTTPConn *_conn,
- const struct addrinfo *_addrs,struct timeb *_start_time){- const struct addrinfo *addr;
- const struct addrinfo *addrs[OP_NPROTOS];
- struct pollfd fds[OP_NPROTOS];
- int ai_family;
- int nprotos;
- int ret;
- int pi;
- int pj;
+ struct addrinfo *_addrs,struct timeb *_start_time){+ struct addrinfo *addr;
+ struct addrinfo *addrs[OP_NPROTOS];
+ struct pollfd fds[OP_NPROTOS];
+ int ai_family;
+ int nprotos;
+ int ret;
+ int pi;
+ int pj;
for(pi=0;pi<OP_NPROTOS;pi++)addrs[pi]=NULL;
/*Try connecting via both IPv4 and IPv6 simultaneously, and keep the first
one that succeeds.
@@ -2065,7 +2079,7 @@
}
static int op_http_connect(OpusHTTPStream *_stream,OpusHTTPConn *_conn,
- const struct addrinfo *_addrs,struct timeb *_start_time){+ struct addrinfo *_addrs,struct timeb *_start_time){struct timeb resolve_time;
struct addrinfo *new_addrs;
int ret;
@@ -2138,19 +2152,20 @@
Scheme and append it to the given string buffer.*/
static int op_sb_append_basic_auth_header(OpusStringBuf *_sb,
const char *_header,const char *_user,const char *_pass){- int user_len;
- int pass_len;
- int user_pass_len;
- int base64_len;
- int nbuf_total;
- int ret;
+ size_t user_len;
+ size_t pass_len;
+ int user_pass_len;
+ int base64_len;
+ int nbuf_total;
+ int ret;
ret=op_sb_append_string(_sb,_header);
ret|=op_sb_append(_sb,": Basic ",8);
user_len=strlen(_user);
pass_len=strlen(_pass);
+ if(OP_UNLIKELY(user_len>(size_t)INT_MAX))return OP_EFAULT;
if(OP_UNLIKELY(pass_len>INT_MAX-user_len))return OP_EFAULT;
- if(OP_UNLIKELY(user_len+pass_len>(INT_MAX>>2)*3-3))return OP_EFAULT;
- user_pass_len=user_len+1+pass_len;
+ if(OP_UNLIKELY((int)(user_len+pass_len)>(INT_MAX>>2)*3-3))return OP_EFAULT;
+ user_pass_len=(int)(user_len+pass_len)+1;
base64_len=OP_BASE64_LENGTH(user_pass_len);
/*Stick "user:pass" at the end of the buffer so we can Base64 encode it
in-place.*/
@@ -2160,9 +2175,9 @@
ret|=op_sb_ensure_capacity(_sb,nbuf_total);
if(OP_UNLIKELY(ret<0))return ret;
_sb->nbuf=nbuf_total-user_pass_len;
- OP_ALWAYS_TRUE(!op_sb_append(_sb,_user,user_len));
+ OP_ALWAYS_TRUE(!op_sb_append(_sb,_user,(int)user_len));
OP_ALWAYS_TRUE(!op_sb_append(_sb,":",1));
- OP_ALWAYS_TRUE(!op_sb_append(_sb,_pass,pass_len));
+ OP_ALWAYS_TRUE(!op_sb_append(_sb,_pass,(int)pass_len));
op_base64_encode(_sb->buf+nbuf_total-base64_len,
_sb->buf+nbuf_total-user_pass_len,user_pass_len);
return op_sb_append(_sb,"\r\n",2);
@@ -2781,7 +2796,7 @@
ret=op_http_conn_handle_response(_stream,_conn);
if(OP_UNLIKELY(ret!=0))return OP_FALSE;
ftime(&end_time);
- _stream->cur_conni=_conn-_stream->conns;
+ _stream->cur_conni=(int)(_conn-_stream->conns);
OP_ASSERT(_stream->cur_conni>=0&&_stream->cur_conni<OP_NCONNS_MAX);
/*The connection has been successfully opened.
Update the connection time estimate.*/
@@ -2885,7 +2900,7 @@
content_length=_stream->content_length;
}
OP_ASSERT(end_pos>pos);
- _buf_size=OP_MIN(_buf_size,end_pos-pos);
+ _buf_size=(int)OP_MIN(_buf_size,end_pos-pos);
}
nread=op_http_conn_read(_conn,(char *)_buf,_buf_size,1);
if(OP_UNLIKELY(nread<0))return nread;
@@ -2921,7 +2936,7 @@
static int op_http_stream_read(void *_stream,
unsigned char *_ptr,int _buf_size){OpusHTTPStream *stream;
- ptrdiff_t nread;
+ int nread;
opus_int64 size;
opus_int64 pos;
int ci;
@@ -3125,7 +3140,8 @@
*pnext=conn->next;
conn->next=stream->lru_head;
stream->lru_head=conn;
- stream->cur_conni=conn-stream->conns;
+ stream->cur_conni=(int)(conn-stream->conns);
+ OP_ASSERT(stream->cur_conni>=0&&stream->cur_conni<OP_NCONNS_MAX);
return 0;
}
pnext=&conn->next;
@@ -3177,7 +3193,8 @@
*pnext=conn->next;
conn->next=stream->lru_head;
stream->lru_head=conn;
- stream->cur_conni=conn-stream->conns;
+ stream->cur_conni=(int)(conn-stream->conns);
+ OP_ASSERT(stream->cur_conni>=0&&stream->cur_conni<OP_NCONNS_MAX);
return 0;
}
close_pnext=pnext;
--- a/src/info.c
+++ b/src/info.c
@@ -279,11 +279,11 @@
}
int opus_tags_add(OpusTags *_tags,const char *_tag,const char *_value){- char *comment;
- int tag_len;
- int value_len;
- int ncomments;
- int ret;
+ char *comment;
+ size_t tag_len;
+ size_t value_len;
+ int ncomments;
+ int ret;
ncomments=_tags->comments;
ret=op_tags_ensure_capacity(_tags,ncomments+1);
if(OP_UNLIKELY(ret<0))return ret;
@@ -290,6 +290,8 @@
tag_len=strlen(_tag);
value_len=strlen(_value);
/*+2 for '=' and '\0'.*/
+ if(tag_len+value_len<tag_len)return OP_EFAULT;
+ if(tag_len+value_len>(size_t)INT_MAX-2)return OP_EFAULT;
comment=(char *)_ogg_malloc(sizeof(*comment)*(tag_len+value_len+2));
if(OP_UNLIKELY(comment==NULL))return OP_EFAULT;
memcpy(comment,_tag,sizeof(*comment)*tag_len);
@@ -296,7 +298,7 @@
comment[tag_len]='=';
memcpy(comment+tag_len+1,_value,sizeof(*comment)*(value_len+1));
_tags->user_comments[ncomments]=comment;
- _tags->comment_lengths[ncomments]=tag_len+value_len+1;
+ _tags->comment_lengths[ncomments]=(int)(tag_len+value_len+1);
_tags->comments=ncomments+1;
return 0;
}
@@ -337,7 +339,10 @@
}
int opus_tagcompare(const char *_tag_name,const char *_comment){- return opus_tagncompare(_tag_name,strlen(_tag_name),_comment);
+ size_t tag_len;
+ tag_len=strlen(_tag_name);
+ if(OP_UNLIKELY(tag_len>(size_t)INT_MAX))return -1;
+ return opus_tagncompare(_tag_name,(int)tag_len,_comment);
}
int opus_tagncompare(const char *_tag_name,int _tag_len,const char *_comment){@@ -348,17 +353,18 @@
}
const char *opus_tags_query(const OpusTags *_tags,const char *_tag,int _count){- char **user_comments;
- int tag_len;
- int found;
- int ncomments;
- int ci;
+ char **user_comments;
+ size_t tag_len;
+ int found;
+ int ncomments;
+ int ci;
tag_len=strlen(_tag);
+ if(OP_UNLIKELY(tag_len>(size_t)INT_MAX))return NULL;
ncomments=_tags->comments;
user_comments=_tags->user_comments;
found=0;
for(ci=0;ci<ncomments;ci++){- if(!opus_tagncompare(_tag,tag_len,user_comments[ci])){+ if(!opus_tagncompare(_tag,(int)tag_len,user_comments[ci])){/*We return a pointer to the data, not a copy.*/
if(_count==found++)return user_comments[ci]+tag_len+1;
}
@@ -368,17 +374,18 @@
}
int opus_tags_query_count(const OpusTags *_tags,const char *_tag){- char **user_comments;
- int tag_len;
- int found;
- int ncomments;
- int ci;
+ char **user_comments;
+ size_t tag_len;
+ int found;
+ int ncomments;
+ int ci;
tag_len=strlen(_tag);
+ if(OP_UNLIKELY(tag_len>(size_t)INT_MAX))return 0;
ncomments=_tags->comments;
user_comments=_tags->user_comments;
found=0;
for(ci=0;ci<ncomments;ci++){- if(!opus_tagncompare(_tag,tag_len,user_comments[ci]))found++;
+ if(!opus_tagncompare(_tag,(int)tag_len,user_comments[ci]))found++;
}
return found;
}
@@ -403,7 +410,8 @@
ncomments=_tags->comments;
/*Look for the first valid tag with the name _tag_name and use that.*/
for(ci=0;ci<ncomments;ci++){- if(opus_tagncompare(_tag_name,_tag_len,comments[ci])==0){+ OP_ASSERT(tag_len<=(size_t)INT_MAX);
+ if(opus_tagncompare(_tag_name,(int)_tag_len,comments[ci])==0){char *p;
opus_int32 gain_q8;
int negative;
--- a/src/opusfile.c
+++ b/src/opusfile.c
@@ -86,14 +86,15 @@
This is to prevent us spending a lot of time allocating memory and looking
for Ogg pages in non-Ogg files.*/
if(memcmp(_initial_data,"OggS",4)!=0)return OP_ENOTFORMAT;
+ if(OP_UNLIKELY(_initial_bytes>(size_t)LONG_MAX))return OP_EFAULT;
ogg_sync_init(&oy);
- data=ogg_sync_buffer(&oy,_initial_bytes);
+ data=ogg_sync_buffer(&oy,(long)_initial_bytes);
if(data!=NULL){ogg_stream_state os;
ogg_page og;
int ret;
memcpy(data,_initial_data,_initial_bytes);
- ogg_sync_wrote(&oy,_initial_bytes);
+ ogg_sync_wrote(&oy,(long)_initial_bytes);
ogg_stream_init(&os,-1);
err=OP_FALSE;
do{@@ -1504,6 +1505,7 @@
int seekable;
int ret;
memset(_of,0,sizeof(*_of));
+ if(OP_UNLIKELY(_initial_bytes>(size_t)LONG_MAX))return OP_EFAULT;
_of->end=-1;
_of->source=_source;
*&_of->callbacks=*_cb;
@@ -1520,9 +1522,9 @@
decoding entire files from RAM.*/
if(_initial_bytes>0){char *buffer;
- buffer=ogg_sync_buffer(&_of->oy,_initial_bytes);
+ buffer=ogg_sync_buffer(&_of->oy,(long)_initial_bytes);
memcpy(buffer,_initial_data,_initial_bytes*sizeof(*buffer));
- ogg_sync_wrote(&_of->oy,_initial_bytes);
+ ogg_sync_wrote(&_of->oy,(long)_initial_bytes);
}
/*Can we seek?
Stevens suggests the seek test is portable.*/
--- a/src/wincerts.c
+++ b/src/wincerts.c
@@ -100,7 +100,8 @@
representation for something, it's the answer that 9 of them would
give you back.
I don't think OpenSSL's encoding qualifies.*/
- find_para.cbData=_name->bytes->length;
+ if(OP_UNLIKELY(_name->bytes->length>MAXDWORD))return 0;
+ find_para.cbData=(DWORD)_name->bytes->length;
find_para.pbData=(unsigned char *)_name->bytes->data;
cert=CertFindCertificateInStore(h_store,X509_ASN_ENCODING,0,
CERT_FIND_SUBJECT_NAME,&find_para,NULL);
@@ -122,7 +123,8 @@
ret=op_capi_retrieve_by_subject(_lu,_type,_name,_ret);
if(ret>0)return ret;
memset(&cert_info,0,sizeof(cert_info));
- cert_info.Issuer.cbData=_name->bytes->length;
+ if(OP_UNLIKELY(_name->bytes->length>MAXDWORD))return 0;
+ cert_info.Issuer.cbData=(DWORD)_name->bytes->length;
cert_info.Issuer.pbData=(unsigned char *)_name->bytes->data;
memset(&find_para,0,sizeof(find_para));
find_para.pCertInfo=&cert_info;