use of int and overflow review

diff --git a/base/chiark-tcl.h b/base/chiark-tcl.h
index 5ee975aa072b4975a8b36c5a97ebda7ebad746e2..1ec2e4edbc1e5d91690c02fd31da1d28037a6beb 100644 (file)
--- a/base/chiark-tcl.h
+++ b/base/chiark-tcl.h
@@ -106,7 +106,7 @@ void cht_objfreeir(Tcl_Obj *o);
 int cht_get_urandom(Tcl_Interp *ip, Byte *buffer, int l);
 
 void cht_obj_updatestr_vstringls(Tcl_Obj *o, ...);
-  /* const char*, int, const char*, int, ..., (const char*)0 */
+  /* const char*, size_t, const char*, size_t, ..., (const char*)0 */
 void cht_obj_updatestr_string_len(Tcl_Obj *o, const char *str, int l);
 void cht_obj_updatestr_string(Tcl_Obj *o, const char *str);
 

diff --git a/base/hook.c b/base/hook.c
index e0b16d4fac762d7feb2ddb59948f63b89d7917be..cc3eba867eaf9111e62bab013376f724aee53334 100644 (file)
--- a/base/hook.c
+++ b/base/hook.c
@@ -53,11 +53,15 @@ void cht_obj_updatestr_vstringls(Tcl_Obj *o, ...) {
   va_list al;
   char *p;
   const char *part;
-  int l, pl;
+  int l;
+  size_t pl;
 
   va_start(al,o);
-  for (l=0; (part= va_arg(al, const char*)); )
-    l+= va_arg(al, int);
+  for (l=0; (part= va_arg(al, const char*)); ) {
+    pl= va_arg(al, size_t);
+    assert(pl <= INT_MAX/2 - l);
+    l += pl;
+  }
   va_end(al);
   
   o->length= l;
@@ -65,7 +69,7 @@ void cht_obj_updatestr_vstringls(Tcl_Obj *o, ...) {
 
   va_start(al,o);
   for (p= o->bytes; (part= va_arg(al, const char*)); p += pl) {
-    pl= va_arg(al, int);
+    pl= va_arg(al, size_t);
     memcpy(p, part, pl);
   }
   va_end(al);

diff --git a/base/idtable.c b/base/idtable.c
index 389151e2c5a75096a669c37200f2adbc28c626f7..b228a725f69c8387c7b05fcd80f825468eb0f200 100644 (file)
--- a/base/idtable.c
+++ b/base/idtable.c
@@ -147,6 +147,7 @@ Tcl_Obj *cht_ret_iddata(Tcl_Interp *ip, void *val, const IdDataSpec *idds) {
   if (ix==-1) {
     for (ix=0; ix<assoc->n && assoc->a[ix]; ix++);
     if (ix>=assoc->n) {
+      assert(assoc->n < INT_MAX/4);
       assoc->n += 2;
       assoc->n *= 2;
       assoc->a= TREALLOC(assoc->a, assoc->n*sizeof(*assoc->a));

diff --git a/base/scriptinv.c b/base/scriptinv.c
index 5757d47b52f9ef8549a577ee29f76b7d2a050f78..26d5030f762ed6fd62c5e16eaebd8c17f9403ccc 100644 (file)
--- a/base/scriptinv.c
+++ b/base/scriptinv.c
@@ -45,6 +45,7 @@ int cht_scriptinv_set(ScriptToInvoke *si, Tcl_Interp *ip,
   if (xargs) {
     rc= Tcl_ListObjLength(ip, xargs, &xlength);  if (rc) return rc;
     Tcl_IncrRefCount(xargs);
+    assert(si->llen < INT_MAX/2 && xlength < INT_MAX/2);
     si->llen += xlength;
   }
 

diff --git a/cdb/writeable.c b/cdb/writeable.c
index 6259ea05ea9803a41b619896cc56771774bbbeeb..0df1b2799ae55a59e60672c23e99fe56f97304aa 100644 (file)
--- a/cdb/writeable.c
+++ b/cdb/writeable.c
@@ -20,6 +20,8 @@
 
 #include "chiark_tcl_cdb.h"
 
+#define KEYLEN_MAX (INT_MAX/2)
+
 #define ftello ftell
 #define fseeko fseek
 
@@ -44,7 +46,8 @@ typedef struct Pathbuf {
 #define MAX_SUFFIX 5
 
 static void pathbuf_init(Pathbuf *pb, const char *pathb) {
-  int l= strlen(pathb);
+  size_t l= strlen(pathb);
+  assert(l < INT_MAX);
   pb->buf= TALLOC(l + MAX_SUFFIX + 1);
   memcpy(pb->buf, pathb, l);
   pb->sfx= pb->buf + l;
@@ -250,7 +253,7 @@ static int readlognum(FILE *f, int delim, int *num_r) {
   *p= 0;
 
   errno=0; ul= strtoul(numbuf, &ep, 10);
-  if (*ep || errno || ul >= INT_MAX/2) return -2;
+  if (*ep || errno || ul >= KEYLEN_MAX) return -2;
   *num_r= ul;
   return 0;
 }
@@ -313,7 +316,7 @@ static int readstorelogrecord(FILE *f, HashTable *ht,
 static int writerecord(FILE *f, const char *key, const HashValue *val) {
   int r;
 
-  r= fprintf(f, "+%d,%d:%s->", strlen(key), val->len, key);
+  r= fprintf(f, "+%d,%d:%s->", (int)strlen(key), val->len, key);
   if (r<0) return -1;
   
   r= fwrite(val->data, 1, val->len, f);
@@ -859,6 +862,9 @@ static int update(Tcl_Interp *ip, Rw *rw, const char *key,
   HashValue *val;
   int rc, r;
 
+  if (strlen(key) >= KEYLEN_MAX)
+    return cht_staticerr(ip, "key too long", "CDB KEYOVERFLOW");
+
   if (!rw->logfile) return cht_staticerr
     (ip, "previous compact failed; cdbwr must be closed and reopened "
      "before any further updates", "CDB BROKEN");

diff --git a/debian/changelog b/debian/changelog
index bbf511e34535f510add525ad7be3dbeb51d59a90..64d09f77d19fde495e5173d9c9eb7c8879145104 100644 (file)
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,9 +2,16 @@ chiark-tcl (1.0.2) unstable; urgency=low
 
   Bugfixes:
   * Do not adns_cancel in the middle of adns_forallqueries.
+  * strlen returns size_t, not int; fixed up everywhere relevant.
+    Closes #393970.  (Bug exists only where int and ssize_t differ.)
+
+  Portability fixes:
+  * Remove unecessary assertion of val<=0xffffffffUL where uint32_t val;
+    Closes: #394039 (FTBFS due to unhelpful GCC warning).
 
   Internal improvements:
   * Add a few assertions about *_LLEN in adns.c.
+  * Comprehensive review of use of `int' and defense against overflow.
 
  --
 

diff --git a/dgram/dgram.c b/dgram/dgram.c
index fdb1d3e2bbd84233bee001f2f476e191d362f699..24c5446c0b1625528f98fbb5ce4dceb664845f28 100644 (file)
--- a/dgram/dgram.c
+++ b/dgram/dgram.c
@@ -113,6 +113,7 @@ static void recv_call(ClientData sock_cd, int mask) {
     }
 
     TFREE(sock->msg_buf);
+    assert(sock->msg_buflen < INT_MAX/4);
     sock->msg_buflen *= 2;
     sock->msg_buflen += 100;
     sock->msg_buf= TALLOC(sock->msg_buflen);

diff --git a/hbytes/chop.c b/hbytes/chop.c
index 3e48569ff63bfa343b8984d09a385ba6c7a91338..e649b5aa12a4dce45eb4ade79de62d2347d21c06 100644 (file)
--- a/hbytes/chop.c
+++ b/hbytes/chop.c
@@ -22,13 +22,15 @@
 #include "chiark_tcl_hbytes.h"
 
 static int strs1(Tcl_Interp *ip, int strc, Tcl_Obj *const *strv, int *l_r) {
-  int rc, l, i;
+  int rc, l, i, pl;
 
   l= 0;
   for (i=1; i<strc; i++) {
     rc= Tcl_ConvertToType(ip,strv[i],&cht_hbytes_type);
     if (rc) return rc;
-    l += cht_hb_len(OBJ_HBYTES(strv[i]));
+    pl= cht_hb_len(OBJ_HBYTES(strv[i]));
+    assert(l < INT_MAX/2 && pl < INT_MAX/2);
+    l += pl;
   }
   *l_r= l;
   return TCL_OK;

diff --git a/hbytes/hbytes.c b/hbytes/hbytes.c
index 5daecd1c5038437357fb896f1bd9d4e4b9044005..15980dcd6024e7518c8f60a7c1ba72bfb295f3d3 100644 (file)
--- a/hbytes/hbytes.c
+++ b/hbytes/hbytes.c
@@ -98,6 +98,8 @@ Byte *cht_hb_prepend(HBytes_Value *hb, int el) {
   Byte *old_block, *new_block, *new_dstart;
 
   cx= complex(hb);
+
+  assert(el < INT_MAX/4 && cx->len < INT_MAX/2);
   
   if (cx->prespace < el) {
     new_prespace= el*2 + cx->len;
@@ -121,6 +123,7 @@ Byte *cht_hb_append(HBytes_Value *hb, int el) {
   Byte *newpart, *new_block, *old_block;
 
   cx= complex(hb);
+  assert(el < INT_MAX/4 && cx->len < INT_MAX/4);
 
   new_len= cx->len + el;
   if (new_len > cx->avail) {

diff --git a/hbytes/hook.c b/hbytes/hook.c
index b49aaa1a8b9414c5de31a2193e2a23bc950e4d08..b2d4dc4e1abc657da57497315d8999b1795a2363 100644 (file)
--- a/hbytes/hook.c
+++ b/hbytes/hook.c
@@ -73,6 +73,7 @@ void cht_obj_updatestr_array_prefix(Tcl_Obj *o, const Byte *byte,
   int pl;
 
   pl= strlen(prefix);
+  assert(l < INT_MAX/2 - 1 - pl);
   o->length= l*2+pl;
   str= o->bytes= TALLOC(o->length+1);
   

diff --git a/hbytes/ulongs.c b/hbytes/ulongs.c
index 5ab364ef33d248f788f8fe737323ded0ab9fe310..f6afa3f6c5a1b5279749e5b38125c250980af655 100644 (file)
--- a/hbytes/ulongs.c
+++ b/hbytes/ulongs.c
@@ -281,10 +281,7 @@ static void ulong_t_ustr(Tcl_Obj *o) {
   char buf[9];
 
   val= *(const uint32_t*)&o->internalRep.longValue;
-
-  assert(val <= 0xffffffffUL);
   snprintf(buf,sizeof(buf), "%08lx", (unsigned long)val);
-
   cht_obj_updatestr_vstringls(o, buf, sizeof(buf)-1, (char*)0);
 }
 

diff --git a/maskmap/addrmap.c b/maskmap/addrmap.c
index 437b41ae1c8c12050bb0fea07b75e2eae3d62dd7..c0f66288c194a8a5bb9909a4fa290b00d895ad7e 100644 (file)
--- a/maskmap/addrmap.c
+++ b/maskmap/addrmap.c
@@ -93,6 +93,7 @@ static void am_reallocentries(AddrMap_Value *am, int len) {
   assert(len >= am->space);
   if (!len) return;
 
+  assert(len < INT_MAX/sizeof(*newentries));
   newentries= TREALLOC(am->entries, sizeof(*newentries)*len);
   assert(newentries);
   
@@ -236,6 +237,7 @@ static int addrmap_t_sfa(Tcl_Interp *ip, Tcl_Obj *o) {
   }
 
   am->byl= bitlen/8;
+  assert(inlen < INT_MAX/2);
   am_reallocentries(am, (inlen-1)*2+1);
 
   ame= ame_sfa_alloc(am);

diff --git a/maskmap/maskmap.c b/maskmap/maskmap.c
index ad141a925ca38123224a0cd21cb0ccd2b420cdff..8fa6dc2030696f75c60164160eb3effbca98b43f 100644 (file)
--- a/maskmap/maskmap.c
+++ b/maskmap/maskmap.c
@@ -281,6 +281,7 @@ int do_addrmap_amend(ClientData cd, Tcl_Interp *ip,
 
     breaking= &am.entries[searched];
     nreplacements= new.prefix - breaking->prefixlen + 1;
+    fixme check integer overflow ^
     replacements= TALLOC(sizeof(*replacements) * nreplacements);
 
     for (fragmentlen= breaking->prefixlen + 1,

diff --git a/tuntap/tuntap.c b/tuntap/tuntap.c
index 9486a9add8b871ca4665099322b4ce19c886973c..b1bad35567c66ed807b97920fc8389911cfb4e88 100644 (file)
--- a/tuntap/tuntap.c
+++ b/tuntap/tuntap.c
@@ -89,7 +89,8 @@ static void cancel(TuntapSocket *sock) {
 static void read_call(ClientData sock_cd, int mask) {
   TuntapSocket *sock= (void*)sock_cd;
   Tcl_Interp *ip= sock->ip;
-  int sz, rc;
+  int rc;
+  ssize_t sz;
   HBytes_Value message_val;
   Tcl_Obj *args[2];