Tighten up a lot of casts from unsigned to int which are read by one
authorsimon <simon@cda61777-01e9-0310-a592-d414129be87e>
Sun, 14 Jul 2013 10:45:54 +0000 (10:45 +0000)
committersimon <simon@cda61777-01e9-0310-a592-d414129be87e>
Sun, 14 Jul 2013 10:45:54 +0000 (10:45 +0000)
of the GET_32BIT macros and then used as length fields. Missing bounds
checks against zero have been added, and also I've introduced a helper
function toint() which casts from unsigned to int in such a way as to
avoid C undefined behaviour, since I'm not sure I trust compilers any
more to do the obviously sensible thing.

git-svn-id: svn://svn.tartarus.org/sgt/putty@9918 cda61777-01e9-0310-a592-d414129be87e

conf.c
import.c
misc.c
misc.h
sftp.c
ssh.c
sshdss.c
sshpubk.c
sshrsa.c
unix/uxagentc.c
windows/winpgnt.c

diff --git a/conf.c b/conf.c
index 7b6a013..e80f585 100644 (file)
--- a/conf.c
+++ b/conf.c
@@ -522,14 +522,15 @@ int conf_deserialise(Conf *conf, void *vdata, int maxsize)
     unsigned char *data = (unsigned char *)vdata;
     unsigned char *start = data;
     struct conf_entry *entry;
-    int primary, used;
+    unsigned primary;
+    int used;
     unsigned char *zero;
 
     while (maxsize >= 4) {
        primary = GET_32BIT_MSB_FIRST(data);
        data += 4, maxsize -= 4;
 
-       if ((unsigned)primary >= N_CONFIG_OPTIONS)
+       if (primary >= N_CONFIG_OPTIONS)
            break;
 
        entry = snew(struct conf_entry);
@@ -541,7 +542,7 @@ int conf_deserialise(Conf *conf, void *vdata, int maxsize)
                sfree(entry);
                goto done;
            }
-           entry->key.secondary.i = GET_32BIT_MSB_FIRST(data);
+           entry->key.secondary.i = toint(GET_32BIT_MSB_FIRST(data));
            data += 4, maxsize -= 4;
            break;
          case TYPE_STR:
@@ -564,7 +565,7 @@ int conf_deserialise(Conf *conf, void *vdata, int maxsize)
                sfree(entry);
                goto done;
            }
-           entry->value.u.intval = GET_32BIT_MSB_FIRST(data);
+           entry->value.u.intval = toint(GET_32BIT_MSB_FIRST(data));
            data += 4, maxsize -= 4;
            break;
          case TYPE_STR:
index ce957bc..d009241 100644 (file)
--- a/import.c
+++ b/import.c
@@ -289,7 +289,7 @@ static int ssh2_read_mpint(void *data, int len, struct mpint_pos *ret)
 
     if (len < 4)
         goto error;
-    bytes = GET_32BIT(d);
+    bytes = toint(GET_32BIT(d));
     if (bytes < 0 || len-4 < bytes)
         goto error;
 
@@ -745,6 +745,10 @@ int openssh_write(const Filename *filename, struct ssh2_userkey *key,
         struct mpint_pos n, e, d, p, q, iqmp, dmp1, dmq1;
         Bignum bd, bp, bq, bdmp1, bdmq1;
 
+        /*
+         * These blobs were generated from inside PuTTY, so we needn't
+         * treat them as untrusted.
+         */
         pos = 4 + GET_32BIT(pubblob);
         pos += ssh2_read_mpint(pubblob+pos, publen-pos, &e);
         pos += ssh2_read_mpint(pubblob+pos, publen-pos, &n);
@@ -798,6 +802,10 @@ int openssh_write(const Filename *filename, struct ssh2_userkey *key,
         int pos;
         struct mpint_pos p, q, g, y, x;
 
+        /*
+         * These blobs were generated from inside PuTTY, so we needn't
+         * treat them as untrusted.
+         */
         pos = 4 + GET_32BIT(pubblob);
         pos += ssh2_read_mpint(pubblob+pos, publen-pos, &p);
         pos += ssh2_read_mpint(pubblob+pos, publen-pos, &q);
@@ -1216,11 +1224,12 @@ int sshcom_encrypted(const Filename *filename, char **comment)
     pos = 8;
     if (key->keyblob_len < pos+4)
         goto done;                     /* key is far too short */
-    pos += 4 + GET_32BIT(key->keyblob + pos);   /* skip key type */
-    if (key->keyblob_len < pos+4)
+    len = toint(GET_32BIT(key->keyblob + pos));
+    if (len < 0 || len > key->keyblob_len - pos - 4)
         goto done;                     /* key is far too short */
-    len = GET_32BIT(key->keyblob + pos);   /* find cipher-type length */
-    if (key->keyblob_len < pos+4+len)
+    pos += 4 + len;                    /* skip key type */
+    len = toint(GET_32BIT(key->keyblob + pos)); /* find cipher-type length */
+    if (len < 0 || len > key->keyblob_len - pos - 4)
         goto done;                     /* cipher type string is incomplete */
     if (len != 4 || 0 != memcmp(key->keyblob + pos + 4, "none", 4))
         answer = 1;
@@ -1236,8 +1245,7 @@ int sshcom_encrypted(const Filename *filename, char **comment)
 
 static int sshcom_read_mpint(void *data, int len, struct mpint_pos *ret)
 {
-    int bits;
-    int bytes;
+    unsigned bits, bytes;
     unsigned char *d = (unsigned char *) data;
 
     if (len < 4)
@@ -1309,7 +1317,8 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase,
      */
     pos = 8;
     if (key->keyblob_len < pos+4 ||
-        (len = GET_32BIT(key->keyblob + pos)) > key->keyblob_len - pos - 4) {
+        (len = toint(GET_32BIT(key->keyblob + pos))) < 0 ||
+        len > key->keyblob_len - pos - 4) {
         errmsg = "key blob does not contain a key type string";
         goto error;
     }
@@ -1329,7 +1338,8 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase,
      * Determine the cipher type.
      */
     if (key->keyblob_len < pos+4 ||
-        (len = GET_32BIT(key->keyblob + pos)) > key->keyblob_len - pos - 4) {
+        (len = toint(GET_32BIT(key->keyblob + pos))) < 0 ||
+        len > key->keyblob_len - pos - 4) {
         errmsg = "key blob does not contain a cipher type string";
         goto error;
     }
@@ -1347,7 +1357,8 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase,
      * Get hold of the encrypted part of the key.
      */
     if (key->keyblob_len < pos+4 ||
-        (len = GET_32BIT(key->keyblob + pos)) > key->keyblob_len - pos - 4) {
+        (len = toint(GET_32BIT(key->keyblob + pos))) < 0 ||
+        len > key->keyblob_len - pos - 4) {
         errmsg = "key blob does not contain actual key data";
         goto error;
     }
@@ -1411,7 +1422,7 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase,
     /*
      * Strip away the containing string to get to the real meat.
      */
-    len = GET_32BIT(ciphertext);
+    len = toint(GET_32BIT(ciphertext));
     if (len < 0 || len > cipherlen-4) {
         errmsg = "containing string was ill-formed";
         goto error;
@@ -1540,6 +1551,10 @@ int sshcom_write(const Filename *filename, struct ssh2_userkey *key,
         int pos;
         struct mpint_pos n, e, d, p, q, iqmp;
 
+        /*
+         * These blobs were generated from inside PuTTY, so we needn't
+         * treat them as untrusted.
+         */
         pos = 4 + GET_32BIT(pubblob);
         pos += ssh2_read_mpint(pubblob+pos, publen-pos, &e);
         pos += ssh2_read_mpint(pubblob+pos, publen-pos, &n);
@@ -1565,6 +1580,10 @@ int sshcom_write(const Filename *filename, struct ssh2_userkey *key,
         int pos;
         struct mpint_pos p, q, g, y, x;
 
+        /*
+         * These blobs were generated from inside PuTTY, so we needn't
+         * treat them as untrusted.
+         */
         pos = 4 + GET_32BIT(pubblob);
         pos += ssh2_read_mpint(pubblob+pos, publen-pos, &p);
         pos += ssh2_read_mpint(pubblob+pos, publen-pos, &q);
diff --git a/misc.c b/misc.c
index 054b793..b4fff66 100644 (file)
--- a/misc.c
+++ b/misc.c
@@ -208,6 +208,29 @@ void burnstr(char *string)             /* sfree(str), only clear it first */
     }
 }
 
+int toint(unsigned u)
+{
+    /*
+     * Convert an unsigned to an int, without running into the
+     * undefined behaviour which happens by the strict C standard if
+     * the value overflows. You'd hope that sensible compilers would
+     * do the sensible thing in response to a cast, but actually I
+     * don't trust modern compilers not to do silly things like
+     * assuming that _obviously_ you wouldn't have caused an overflow
+     * and so they can elide an 'if (i < 0)' test immediately after
+     * the cast.
+     *
+     * Sensible compilers ought of course to optimise this entire
+     * function into 'just return the input value'!
+     */
+    if (u <= (unsigned)INT_MAX)
+        return (int)u;
+    else if (u >= (unsigned)INT_MIN)   /* wrap in cast _to_ unsigned is OK */
+        return INT_MIN + (int)(u - (unsigned)INT_MIN);
+    else
+        return INT_MIN; /* fallback; should never occur on binary machines */
+}
+
 /*
  * Do an sprintf(), but into a custom-allocated buffer.
  * 
diff --git a/misc.h b/misc.h
index 2ca2d8e..eff4a5e 100644 (file)
--- a/misc.h
+++ b/misc.h
@@ -30,6 +30,8 @@ char *dupprintf(const char *fmt, ...);
 char *dupvprintf(const char *fmt, va_list ap);
 void burnstr(char *string);
 
+int toint(unsigned);
+
 char *fgetline(FILE *fp);
 
 void base64_encode_atom(unsigned char *data, int n, char *out);
diff --git a/sftp.c b/sftp.c
index 927f4d9..bf75779 100644 (file)
--- a/sftp.c
+++ b/sftp.c
@@ -150,7 +150,7 @@ static int sftp_pkt_getstring(struct sftp_packet *pkt,
     *p = NULL;
     if (pkt->length - pkt->savedpos < 4)
        return 0;
-    *length = GET_32BIT(pkt->data + pkt->savedpos);
+    *length = toint(GET_32BIT(pkt->data + pkt->savedpos));
     pkt->savedpos += 4;
     if ((int)(pkt->length - pkt->savedpos) < *length || *length < 0) {
        *length = 0;
diff --git a/ssh.c b/ssh.c
index 30ee4ba..ce3a527 100644 (file)
--- a/ssh.c
+++ b/ssh.c
@@ -1449,7 +1449,8 @@ static struct Packet *ssh2_rdpkt(Ssh ssh, unsigned char **data, int *datalen)
            /* See if that gives us a valid packet. */
            if (ssh->scmac->verresult(ssh->sc_mac_ctx,
                                      st->pktin->data + st->packetlen) &&
-               (st->len = GET_32BIT(st->pktin->data)) + 4 == st->packetlen)
+               ((st->len = toint(GET_32BIT(st->pktin->data))) ==
+                 st->packetlen-4))
                    break;
            if (st->packetlen >= OUR_V2_PACKETLIMIT) {
                bombout(("No valid incoming packet found"));
@@ -1482,7 +1483,7 @@ static struct Packet *ssh2_rdpkt(Ssh ssh, unsigned char **data, int *datalen)
        /*
         * Now get the length figure.
         */
-       st->len = GET_32BIT(st->pktin->data);
+       st->len = toint(GET_32BIT(st->pktin->data));
 
        /*
         * _Completely_ silly lengths should be stomped on before they
@@ -2313,7 +2314,7 @@ static void ssh_pkt_getstring(struct Packet *pkt, char **p, int *length)
     *length = 0;
     if (pkt->length - pkt->savedpos < 4)
        return;
-    len = GET_32BIT(pkt->body + pkt->savedpos);
+    len = toint(GET_32BIT(pkt->body + pkt->savedpos));
     if (len < 0)
        return;
     *length = len;
@@ -2397,7 +2398,7 @@ static void ssh2_add_sigblob(Ssh ssh, struct Packet *pkt,
      * See if this is in fact an ssh-rsa signature and a buggy
      * server; otherwise we can just do this the easy way.
      */
-    if ((ssh->remote_bugs & BUG_SSH2_RSA_PADDING) &&
+    if ((ssh->remote_bugs & BUG_SSH2_RSA_PADDING) && pkblob_len > 4+7+4 &&
        (GET_32BIT(pkblob) == 7 && !memcmp(pkblob+4, "ssh-rsa", 7))) {
        int pos, len, siglen;
 
@@ -2406,8 +2407,15 @@ static void ssh2_add_sigblob(Ssh ssh, struct Packet *pkt,
         */
 
        pos = 4+7;                     /* skip over "ssh-rsa" */
-       pos += 4 + GET_32BIT(pkblob+pos);   /* skip over exponent */
-       len = GET_32BIT(pkblob+pos);   /* find length of modulus */
+        len = toint(GET_32BIT(pkblob+pos)); /* get length of exponent */
+        if (len < 0 || len > pkblob_len - pos - 4)
+            goto give_up;
+       pos += 4 + len;                /* skip over exponent */
+        if (pkblob_len - pos < 4)
+            goto give_up;
+       len = toint(GET_32BIT(pkblob+pos)); /* find length of modulus */
+        if (len < 0 || len > pkblob_len - pos - 4)
+            goto give_up;
        pos += 4;                      /* find modulus itself */
        while (len > 0 && pkblob[pos] == 0)
            len--, pos++;
@@ -2417,7 +2425,11 @@ static void ssh2_add_sigblob(Ssh ssh, struct Packet *pkt,
         * Now find the signature integer.
         */
        pos = 4+7;                     /* skip over "ssh-rsa" */
-       siglen = GET_32BIT(sigblob+pos);
+        if (sigblob_len < pos+4)
+            goto give_up;
+       siglen = toint(GET_32BIT(sigblob+pos));
+        if (siglen != sigblob_len - pos - 4)
+            goto give_up;
        /* debug(("signature length is %d\n", siglen)); */
 
        if (len != siglen) {
@@ -2439,7 +2451,10 @@ static void ssh2_add_sigblob(Ssh ssh, struct Packet *pkt,
            return;
        }
 
-       /* Otherwise fall through and do it the easy way. */
+       /* Otherwise fall through and do it the easy way. We also come
+         * here as a fallback if we discover above that the key blob
+         * is misformatted in some way. */
+      give_up:;
     }
 
     ssh2_pkt_addstring_start(pkt);
@@ -3677,7 +3692,12 @@ static int do_ssh1_login(Ssh ssh, unsigned char *in, int inlen,
            if (s->response && s->responselen >= 5 &&
                s->response[4] == SSH1_AGENT_RSA_IDENTITIES_ANSWER) {
                s->p = s->response + 5;
-               s->nkeys = GET_32BIT(s->p);
+               s->nkeys = toint(GET_32BIT(s->p));
+                if (s->nkeys < 0) {
+                    logeventf(ssh, "Pageant reported negative key count %d",
+                              s->nkeys);
+                    s->nkeys = 0;
+                }
                s->p += 4;
                logeventf(ssh, "Pageant has %d SSH-1 keys", s->nkeys);
                for (s->keyi = 0; s->keyi < s->nkeys; s->keyi++) {
@@ -3687,22 +3707,23 @@ static int do_ssh1_login(Ssh ssh, unsigned char *in, int inlen,
                        int n, ok = FALSE;
                        do {           /* do while (0) to make breaking easy */
                            n = ssh1_read_bignum
-                               (s->p, s->responselen-(s->p-s->response),
+                               (s->p, toint(s->responselen-(s->p-s->response)),
                                 &s->key.exponent);
                            if (n < 0)
                                break;
                            s->p += n;
                            n = ssh1_read_bignum
-                               (s->p, s->responselen-(s->p-s->response),
+                               (s->p, toint(s->responselen-(s->p-s->response)),
                                 &s->key.modulus);
                            if (n < 0)
-                           break;
+                                break;
                            s->p += n;
                            if (s->responselen - (s->p-s->response) < 4)
                                break;
-                           s->commentlen = GET_32BIT(s->p);
+                           s->commentlen = toint(GET_32BIT(s->p));
                            s->p += 4;
-                           if (s->responselen - (s->p-s->response) <
+                           if (s->commentlen < 0 ||
+                                toint(s->responselen - (s->p-s->response)) <
                                s->commentlen)
                                break;
                            s->commentp = (char *)s->p;
@@ -7193,16 +7214,18 @@ static void ssh2_msg_channel_request(Ssh ssh, struct Packet *pktin)
                    is_int = FALSE;
                } else {
                    int maybe_int = FALSE, maybe_str = FALSE;
-#define CHECK_HYPOTHESIS(offset, result) \
-    do { \
-       long q = offset; \
-       if (q >= 0 && q+4 <= len) { \
-           q = q + 4 + GET_32BIT(p+q); \
-           if (q >= 0 && q+4 <= len && \
-                   ((q = q + 4 + GET_32BIT(p+q))!= 0) && q == len) \
-               result = TRUE; \
-       } \
-    } while(0)
+#define CHECK_HYPOTHESIS(offset, result)                                \
+                    do                                                  \
+                    {                                                   \
+                        int q = toint(offset);                          \
+                        if (q >= 0 && q+4 <= len) {                     \
+                            q = toint(q + 4 + GET_32BIT(p+q));          \
+                            if (q >= 0 && q+4 <= len &&                 \
+                                ((q = toint(q + 4 + GET_32BIT(p+q))) != 0) && \
+                                q == len)                               \
+                                result = TRUE;                          \
+                        }                                               \
+                    } while(0)
                    CHECK_HYPOTHESIS(4+1, maybe_int);
                    CHECK_HYPOTHESIS(4+num+1, maybe_str);
 #undef CHECK_HYPOTHESIS
@@ -7896,13 +7919,53 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
                int keyi;
                unsigned char *p;
                p = s->agent_response + 5;
-               s->nkeys = GET_32BIT(p);
+               s->nkeys = toint(GET_32BIT(p));
+
+                /*
+                 * Vet the Pageant response to ensure that the key
+                 * count and blob lengths make sense.
+                 */
+                if (s->nkeys < 0) {
+                    logeventf(ssh, "Pageant response contained a negative"
+                              " key count %d", s->nkeys);
+                    s->nkeys = 0;
+                    goto done_agent_query;
+                } else {
+                    unsigned char *q = p + 4;
+                    int lenleft = s->agent_responselen - 5 - 4;
+
+                    for (keyi = 0; keyi < s->nkeys; keyi++) {
+                        int bloblen, commentlen;
+                        if (lenleft < 4) {
+                            logeventf(ssh, "Pageant response was truncated");
+                            s->nkeys = 0;
+                            goto done_agent_query;
+                        }
+                        bloblen = toint(GET_32BIT(q));
+                        if (bloblen < 0 || bloblen > lenleft) {
+                            logeventf(ssh, "Pageant response was truncated");
+                            s->nkeys = 0;
+                            goto done_agent_query;
+                        }
+                        lenleft -= 4 + bloblen;
+                        q += 4 + bloblen;
+                        commentlen = toint(GET_32BIT(q));
+                        if (commentlen < 0 || commentlen > lenleft) {
+                            logeventf(ssh, "Pageant response was truncated");
+                            s->nkeys = 0;
+                            goto done_agent_query;
+                        }
+                        lenleft -= 4 + commentlen;
+                        q += 4 + commentlen;
+                    }
+                }
+
                p += 4;
                logeventf(ssh, "Pageant has %d SSH-2 keys", s->nkeys);
                if (s->publickey_blob) {
                    /* See if configured key is in agent. */
                    for (keyi = 0; keyi < s->nkeys; keyi++) {
-                       s->pklen = GET_32BIT(p);
+                       s->pklen = toint(GET_32BIT(p));
                        if (s->pklen == s->publickey_bloblen &&
                            !memcmp(p+4, s->publickey_blob,
                                    s->publickey_bloblen)) {
@@ -7913,7 +7976,7 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
                            break;
                        }
                        p += 4 + s->pklen;
-                       p += GET_32BIT(p) + 4; /* comment */
+                       p += toint(GET_32BIT(p)) + 4; /* comment */
                    }
                    if (!s->pkblob_in_agent) {
                        logevent("Configured key file not in Pageant");
@@ -7923,6 +7986,7 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
            } else {
                 logevent("Failed to get reply from Pageant");
            }
+          done_agent_query:;
        }
 
     }
@@ -8174,13 +8238,13 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
                logeventf(ssh, "Trying Pageant key #%d", s->keyi);
 
                /* Unpack key from agent response */
-               s->pklen = GET_32BIT(s->agentp);
+               s->pklen = toint(GET_32BIT(s->agentp));
                s->agentp += 4;
                s->pkblob = (char *)s->agentp;
                s->agentp += s->pklen;
-               s->alglen = GET_32BIT(s->pkblob);
+               s->alglen = toint(GET_32BIT(s->pkblob));
                s->alg = s->pkblob + 4;
-               s->commentlen = GET_32BIT(s->agentp);
+               s->commentlen = toint(GET_32BIT(s->agentp));
                s->agentp += 4;
                s->commentp = (char *)s->agentp;
                s->agentp += s->commentlen;
@@ -8284,7 +8348,9 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
                    s->ret = vret;
                    sfree(s->agentreq);
                    if (s->ret) {
-                       if (s->ret[4] == SSH2_AGENT_SIGN_RESPONSE) {
+                       if (s->retlen >= 9 &&
+                            s->ret[4] == SSH2_AGENT_SIGN_RESPONSE &&
+                            GET_32BIT(s->ret + 5) <= (unsigned)(s->retlen-9)) {
                            logevent("Sending Pageant's response");
                            ssh2_add_sigblob(ssh, s->pktout,
                                             s->pkblob, s->pklen,
index 1f15cee..0484c44 100644 (file)
--- a/sshdss.c
+++ b/sshdss.c
@@ -42,7 +42,7 @@ static void getstring(char **data, int *datalen, char **p, int *length)
     *p = NULL;
     if (*datalen < 4)
        return;
-    *length = GET_32BIT(*data);
+    *length = toint(GET_32BIT(*data));
     if (*length < 0)
         return;
     *datalen -= 4;
index 4db37c2..72aaaa9 100644 (file)
--- a/sshpubk.c
+++ b/sshpubk.c
@@ -73,7 +73,7 @@ static int loadrsakey_main(FILE * fp, struct RSAKey *key, int pub_only,
     i += j;
 
     /* Next, the comment field. */
-    j = GET_32BIT(buf + i);
+    j = toint(GET_32BIT(buf + i));
     i += 4;
     if (j < 0 || len - i < j)
        goto end;
index 163a92b..6403343 100644 (file)
--- a/sshrsa.c
+++ b/sshrsa.c
@@ -525,7 +525,7 @@ static void getstring(char **data, int *datalen, char **p, int *length)
     *p = NULL;
     if (*datalen < 4)
        return;
-    *length = GET_32BIT(*data);
+    *length = toint(GET_32BIT(*data));
     if (*length < 0)
         return;
     *datalen -= 4;
index 9f579cd..5734a7b 100644 (file)
@@ -75,13 +75,12 @@ static int agent_select_result(int fd, int event)
     }
     conn->retlen += ret;
     if (conn->retsize == 4 && conn->retlen == 4) {
-       conn->retsize = GET_32BIT(conn->retbuf);
+       conn->retsize = toint(GET_32BIT(conn->retbuf) + 4);
        if (conn->retsize <= 0) {
            conn->retbuf = NULL;
            conn->retlen = 0;
            goto done;
        }
-       conn->retsize += 4;
        assert(conn->retbuf == conn->sizebuf);
        conn->retbuf = snewn(conn->retsize, char);
        memcpy(conn->retbuf, conn->sizebuf, 4);
index 21a69cc..e018159 100644 (file)
@@ -450,7 +450,12 @@ static void add_keyfile(Filename *filename)
                           MB_OK | MB_ICONERROR);
                return;
            }
-           nkeys = GET_32BIT(keylist);
+           nkeys = toint(GET_32BIT(keylist));
+           if (nkeys < 0) {
+               MessageBox(NULL, "Received broken key list?!", APPNAME,
+                          MB_OK | MB_ICONERROR);
+               return;
+           }
            p = keylist + 4;
            keylistlen -= 4;
 
@@ -478,8 +483,8 @@ static void add_keyfile(Filename *filename)
                                   MB_OK | MB_ICONERROR);
                        return;
                    }
-                   n = 4 + GET_32BIT(p);
-                   if (keylistlen < n) {
+                   n = toint(4 + GET_32BIT(p));
+                   if (n < 0 || keylistlen < n) {
                        MessageBox(NULL, "Received broken key list?!", APPNAME,
                                   MB_OK | MB_ICONERROR);
                        return;
@@ -495,8 +500,8 @@ static void add_keyfile(Filename *filename)
                                   MB_OK | MB_ICONERROR);
                        return;
                    }
-                   n = 4 + GET_32BIT(p);
-                   if (keylistlen < n) {
+                   n = toint(4 + GET_32BIT(p));
+                   if (n < 0 || keylistlen < n) {
                        MessageBox(NULL, "Received broken key list?!", APPNAME,
                                   MB_OK | MB_ICONERROR);
                        return;
@@ -998,17 +1003,17 @@ static void answer_msg(void *msg)
 
            if (msgend < p+4)
                goto failure;
-           b.len = GET_32BIT(p);
+           b.len = toint(GET_32BIT(p));
+            if (b.len < 0 || b.len > msgend - (p+4))
+                goto failure;
            p += 4;
-           if (msgend < p+b.len)
-               goto failure;
            b.blob = p;
            p += b.len;
            if (msgend < p+4)
                goto failure;
-           datalen = GET_32BIT(p);
+           datalen = toint(GET_32BIT(p));
            p += 4;
-           if (msgend < p+datalen)
+           if (datalen < 0 || datalen > msgend - p)
                goto failure;
            data = p;
            key = find234(ssh2keys, &b, cmpkeys_ssh2_asymm);
@@ -1081,9 +1086,9 @@ static void answer_msg(void *msg)
                sfree(key);
                goto failure;
            }
-            commentlen = GET_32BIT(p);
+            commentlen = toint(GET_32BIT(p));
 
-           if (msgend < p+commentlen) {
+           if (commentlen < 0 || commentlen > msgend - p) {
                freersakey(key);
                sfree(key);
                goto failure;
@@ -1120,9 +1125,9 @@ static void answer_msg(void *msg)
 
            if (msgend < p+4)
                goto failure;
-           alglen = GET_32BIT(p);
+           alglen = toint(GET_32BIT(p));
            p += 4;
-           if (msgend < p+alglen)
+           if (alglen < 0 || alglen > msgend - p)
                goto failure;
            alg = p;
            p += alglen;
@@ -1156,10 +1161,10 @@ static void answer_msg(void *msg)
                sfree(key);
                goto failure;
            }
-           commlen = GET_32BIT(p);
+           commlen = toint(GET_32BIT(p));
            p += 4;
 
-           if (msgend < p+commlen) {
+           if (commlen < 0 || commlen > msgend - p) {
                key->alg->freekey(key->data);
                sfree(key);
                goto failure;
@@ -1223,10 +1228,10 @@ static void answer_msg(void *msg)
 
            if (msgend < p+4)
                goto failure;
-           b.len = GET_32BIT(p);
+           b.len = toint(GET_32BIT(p));
            p += 4;
 
-           if (msgend < p+b.len)
+           if (b.len < 0 || b.len > msgend - p)
                goto failure;
            b.blob = p;
            p += b.len;