In various places we add and increment integers, hoping that they
don't overflow. We also prepend and append things to our internal
buffer, which is of fixed size, without checking that they will fit.
This means that malicious configuration (for example, long site names)
might be able to take over the secnet program.
So, add a whole lot of checking. Many of these places don't have a
sensible way to return an error; in those cases we assert. Some of
the checks are off-by-one in the sense that they say "assert(x<...)"
when "<=" would be OK too. This is done to avoid having to think too
hard about fenceposts, as it's a simple way to avoid introducing bugs.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
/* #define DUMP_PARSE_TREE */
#include "secnet.h"
/* #define DUMP_PARSE_TREE */
#include "secnet.h"
+#include <assert.h>
+#include <limits.h>
#include <stdio.h>
#include <string.h>
#include "conffile.h"
#include <stdio.h>
#include <string.h>
#include "conffile.h"
default: printf("**unknown primitive type**\n"); break;
}
} else {
default: printf("**unknown primitive type**\n"); break;
}
} else {
printf("%s: (%s line %d)\n",ntype(n->type),n->loc.file,n->loc.line);
ptree_indent(d);
printf(" |-"); ptree_dump(n->l, d+1);
printf("%s: (%s line %d)\n",ntype(n->type),n->loc.file,n->loc.line);
ptree_indent(d);
printf(" |-"); ptree_dump(n->l, d+1);
{
uint32_t l=0;
list_t *i;
{
uint32_t l=0;
list_t *i;
- for (i=a; i; i=i->next) l++;
+ for (i=a; i; i=i->next) { assert(l < INT_MAX); l++; }
if (i->type!=t_string) {
cfgfatal(loc,desc,"\"%s\" must be a string\n",key);
}
if (i->type!=t_string) {
cfgfatal(loc,desc,"\"%s\" must be a string\n",key);
}
+ if (strlen(i->data.string) > INT_MAX/10) {
+ cfgfatal(loc,desc,"\"%s\" is unreasonably long\n",key);
+ }
r=i->data.string;
return r;
}
r=i->data.string;
return r;
}
if (i->type!=t_number) {
cfgfatal(loc,desc,"\"%s\" must be a number\n",key);
}
if (i->type!=t_number) {
cfgfatal(loc,desc,"\"%s\" must be a number\n",key);
}
+ if (i->data.number >= 0x80000000) {
+ cfgfatal(loc,desc,"\"%s\" must fit into a 32-bit signed integer\n",key);
+ }
r=i->data.number;
return r;
}
r=i->data.number;
return r;
}
%option never-interactive
%{
%option never-interactive
%{
+#include <assert.h>
+#include <limits.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
Message(M_FATAL,"config file %s line %d: %s\n",config_file,
config_lineno,"``include'' requires a filename");
BEGIN(INITIAL);
Message(M_FATAL,"config file %s line %d: %s\n",config_file,
config_lineno,"``include'' requires a filename");
BEGIN(INITIAL);
+ assert(config_lineno < INT_MAX);
++config_lineno;
++yynerrs;
}
++config_lineno;
++yynerrs;
}
#include <stdio.h>
#include <gmp.h>
#include <stdio.h>
#include <gmp.h>
#include "secnet.h"
#include "util.h"
#include "secnet.h"
#include "util.h"
cfgfatal(loc,"diffie-hellman","modulus must be a prime\n");
}
}
cfgfatal(loc,"diffie-hellman","modulus must be a prime\n");
}
}
- st->ops.len=mpz_sizeinbase(&st->p,2)/8;
+
+ size_t sz=mpz_sizeinbase(&st->p,2)/8;
+ if (sz>INT_MAX) {
+ cfgfatal(loc,"diffie-hellman","modulus far too large\n");
+ }
+ if (mpz_cmp(&st->g,&st->p) >= 0) {
+ cfgfatal(loc,"diffie-hellman","generator must be less than modulus\n");
+ }
+
+ st->ops.len=sz;
return new_closure(&st->cl);
}
return new_closure(&st->cl);
}
inspired by the 'ipaddr.py' library from Cendio Systems AB. */
#include "secnet.h"
inspired by the 'ipaddr.py' library from Cendio Systems AB. */
#include "secnet.h"
+#include <limits.h>
+#include <assert.h>
#include <stdio.h>
#include <string.h>
#include "ipaddr.h"
#include <stdio.h>
#include <string.h>
#include "ipaddr.h"
uint32_t na;
if (l>a->alloc) {
uint32_t na;
if (l>a->alloc) {
+ assert(a->alloc < (int)(INT_MAX/sizeof(*nd))-EXTEND_ALLOC_BY);
na=a->alloc+EXTEND_ALLOC_BY;
nd=realloc(a->list,sizeof(*nd)*na);
if (!nd) {
na=a->alloc+EXTEND_ALLOC_BY;
nd=realloc(a->list,sizeof(*nd)*na);
if (!nd) {
void subnet_list_append(struct subnet_list *a, uint32_t prefix, uint32_t len)
{
struct subnet *sn;
void subnet_list_append(struct subnet_list *a, uint32_t prefix, uint32_t len)
{
struct subnet *sn;
+ assert(a->entries < INT_MAX);
subnet_list_set_len(a,a->entries+1);
sn=&a->list[a->entries-1];
sn->prefix=prefix;
subnet_list_set_len(a,a->entries+1);
sn=&a->list[a->entries-1];
sn->prefix=prefix;
uint32_t na;
if (l>a->a) {
uint32_t na;
if (l>a->a) {
+ assert(a->a < INT_MAX-EXTEND_ALLOC_BY);
na=a->a+EXTEND_ALLOC_BY;
nd=realloc(a->d,sizeof(*nd)*na);
if (!nd) {
na=a->a+EXTEND_ALLOC_BY;
nd=realloc(a->d,sizeof(*nd)*na);
if (!nd) {
if (secnet_is_daemon) {
/* Messages go to the system log interface */
bp=strlen(buff);
if (secnet_is_daemon) {
/* Messages go to the system log interface */
bp=strlen(buff);
+ assert(bp < MESSAGE_BUFLEN);
vsnprintf(buff+bp,MESSAGE_BUFLEN-bp,message,args);
/* Each line is sent separately */
while ((nlp=strchr(buff,'\n'))) {
vsnprintf(buff+bp,MESSAGE_BUFLEN-bp,message,args);
/* Each line is sent separately */
while ((nlp=strchr(buff,'\n'))) {
+#include <assert.h>
+#include <limits.h>
#include "secnet.h"
#include "util.h"
#include "ipaddr.h"
#include "secnet.h"
#include "util.h"
#include "ipaddr.h"
"netlink_phase_hook");
/* Fill the table */
i=0;
"netlink_phase_hook");
/* Fill the table */
i=0;
- for (c=st->clients; c; c=c->next)
+ for (c=st->clients; c; c=c->next) {
+ assert(i<INT_MAX);
/* Sort the table in descending order of priority */
qsort(st->routes,st->n_clients,sizeof(*st->routes),
netlink_compare_client_priority);
/* Sort the table in descending order of priority */
qsort(st->routes,st->n_clients,sizeof(*st->routes),
netlink_compare_client_priority);
c->kup=False;
c->next=st->clients;
st->clients=c;
c->kup=False;
c->next=st->clients;
st->clients=c;
+ assert(st->n_clients < INT_MAX);
st->n_clients++;
return &c->cl;
st->n_clients++;
return &c->cl;
#include "secnet.h"
#include <stdio.h>
#include "secnet.h"
#include <stdio.h>
+#include <assert.h>
+#include <limits.h>
#include <string.h>
#include <getopt.h>
#include <errno.h>
#include <string.h>
#include <getopt.h>
#include <errno.h>
i->max_nfds=max_nfds;
i->nfds=0;
i->desc=desc;
i->max_nfds=max_nfds;
i->nfds=0;
i->desc=desc;
+ assert(total_nfds < INT_MAX - max_nfds);
total_nfds+=max_nfds;
i->next=reg;
reg=i;
total_nfds+=max_nfds;
i->next=reg;
reg=i;
extern uint32_t dict_read_number(dict_t *dict, cstring_t key, bool_t required,
cstring_t desc, struct cloc loc,
uint32_t def);
extern uint32_t dict_read_number(dict_t *dict, cstring_t key, bool_t required,
cstring_t desc, struct cloc loc,
uint32_t def);
+ /* return value can safely be assigned to int32_t */
extern bool_t dict_read_bool(dict_t *dict, cstring_t key, bool_t required,
cstring_t desc, struct cloc loc, bool_t def);
struct flagstr {
extern bool_t dict_read_bool(dict_t *dict, cstring_t key, bool_t required,
cstring_t desc, struct cloc loc, bool_t def);
struct flagstr {
sprintf(st->tunname,"%s<->%s",st->localname,st->remotename);
/* The information we expect to see in incoming messages of type 1 */
sprintf(st->tunname,"%s<->%s",st->localname,st->remotename);
/* The information we expect to see in incoming messages of type 1 */
+ /* fixme: lots of unchecked overflows here, but the results are only
+ corrupted packets rather than undefined behaviour */
st->setupsiglen=strlen(st->remotename)+strlen(st->localname)+8;
st->setupsig=safe_malloc(st->setupsiglen,"site_apply");
put_uint32(st->setupsig+0,LABEL_MSG1);
st->setupsiglen=strlen(st->remotename)+strlen(st->localname)+8;
st->setupsig=safe_malloc(st->setupsiglen,"site_apply");
put_uint32(st->setupsig+0,LABEL_MSG1);
void *buf_append(struct buffer_if *buf, uint32_t amount) {
void *p;
void *buf_append(struct buffer_if *buf, uint32_t amount) {
void *p;
+ assert(buf->size <= buf->len - amount);
p=buf->start + buf->size;
buf->size+=amount;
return p;
}
void *buf_prepend(struct buffer_if *buf, uint32_t amount) {
p=buf->start + buf->size;
buf->size+=amount;
return p;
}
void *buf_prepend(struct buffer_if *buf, uint32_t amount) {
+ assert(amount <= buf->start - buf->base);
buf->size+=amount;
return buf->start-=amount;
}
buf->size+=amount;
return buf->start-=amount;
}
uint16_t len;
len=strlen(s);
uint16_t len;
len=strlen(s);
+ /* fixme: if string is longer than 65535, result is a corrupted packet */
buf_append_uint16(buf,len);
memcpy(buf_append(buf,len),s,len);
}
buf_append_uint16(buf,len);
memcpy(buf_append(buf,len),s,len);
}