Tweak bounds checks in pageant_add_keyfile.

When we're going through the response from an SSH agent we asked for a
list of keys, and processing the string lengths in the SSH-2 sequence
of (public blob, comment) pairs, we were adding 4 to each string
length, and although we checked if the result came out to a negative
value (if interpreted as a signed integer) or a positive one going
beyond the end of the response buffer, we didn't check if it wrapped
round to a positive value less than 4. As a result, if an agent
returned malformed data sent a length field of 0xFFFFFFFC, the pointer
would advance no distance at all through the buffer, and the next
iteration of the loop would check the same length field again.

(However, this would only consume CPU pointlessly for a limited time,
because the outer loop up to 'nkeys' would still terminate sooner or
later. Also, I don't think this can sensibly be classed as a serious
security hazard - it's arguably a borderline DoS, but it requires a
hostile SSH _agent_ if data of that type is to be sent on purpose, and
an untrusted SSH agent is not part of the normal security model!)

diff --git a/pageant.c b/pageant.c
index 68bfab5309f134f3fe3a60c09d2944b3f20a3e22..31a5540c45f9d36275d532de5fad93d6e6f36b50 100644 (file)
--- a/pageant.c
+++ b/pageant.c
@@ -1344,8 +1344,11 @@ int pageant_add_keyfile(Filename *filename, const char *passphrase,
                         *retstr = dupstr("Received broken key list from agent");
                         return PAGEANT_ACTION_FAILURE;
                    }
-                   n = toint(4 + GET_32BIT(p));
-                   if (n < 0 || keylistlen < n) {
+                   n = GET_32BIT(p);
+                    p += 4;
+                    keylistlen -= 4;
+
+                   if (n < 0 || n > keylistlen) {
                         *retstr = dupstr("Received broken key list from agent");
                         return PAGEANT_ACTION_FAILURE;
                    }
@@ -1359,8 +1362,11 @@ int pageant_add_keyfile(Filename *filename, const char *passphrase,
                         *retstr = dupstr("Received broken key list from agent");
                         return PAGEANT_ACTION_FAILURE;
                    }
-                   n = toint(4 + GET_32BIT(p));
-                   if (n < 0 || keylistlen < n) {
+                   n = GET_32BIT(p);
+                    p += 4;
+                    keylistlen -= 4;
+
+                   if (n < 0 || n > keylistlen) {
                         *retstr = dupstr("Received broken key list from agent");
                         return PAGEANT_ACTION_FAILURE;
                    }