[krbdev.mit.edu #8914] Invalid negative record length in keytab file

[Joshua Neuheisel via RT]

<URL: https://krbdev.mit.edu/rt/Ticket/Display.html?id=8914 >

What about something like this? It keeps the spirit of the second approach without changing the file calls. When reading for the next record, use two fseek calls to skip past these odd records. Ignore them as well when looking for a record to overwrite. It's not a complete patch as it has no tests - and I still need to look for any other places this logic may be lurking.

If nothing else it stops the infinite loop.

--- /krb5-1.18.2.orig/src/lib/krb5/keytab/kt_file.c	2020-05-22 00:21:40.000000000 +0000
+++ /krb5-1.18.2/src/lib/krb5/keytab/kt_file.c	2020-06-15 11:02:30.107034312 +0000
@@ -921,6 +921,16 @@
             size = ntohl(size);
 
         if (size < 0) {
+            /* -INT32_MIN == INT32_MIN */
+
+            if (size == INT32_MIN) {
+                if (fseek(KTFILEP(id), 1, SEEK_CUR)) {
+                    return errno;
+                }
+
+		size++;
+            }
+
             if (fseek(KTFILEP(id), -size, SEEK_CUR)) {
                 return errno;
             }
@@ -1352,6 +1362,14 @@
                 *size_needed = size;
                 break;
             } else {
+                /* -INT32_MIN == INT32_MIN; skip these unusual records */
+                if (size == INT32_MIN) {
+                    if (fseek(fp, 1, SEEK_CUR)) {
+                        return errno;
+                    }
+
+		    size = -(size + 1);
+                }
                 if (fseek(fp, size, SEEK_CUR))
                     return errno;
             }

On 6/12/20, 18:38, "Greg Hudson via RT" <rt at krbdev.mit.edu> wrote:

    External Email - Use Caution

    There are two reasonable ways to react to a length value of -2^31: throw an
    error because the size of the purported hole is absurdly large, or skip forward
    2^31 bytes. (The value is absurd because an entry is only a little bit longer
    than the marshalled principal name, and a principal name shouldn't marshal to
    anywhere near 2^31 bytes.)

    To throw an error we can simply add a comparison to INT32_MIN.

    Handling the value as stated would require more work. If we could assume that
    long is 64 bits, it would be fairly easy; we'd just have to adjust the
    marshalling code so that the variable size is of type long instead of int32_t.
    But we can't assume that long is 64 bits, even in the steady state (long is
    still 32 bits on 64-bit Windows), so we'd have to abandon stdio and use POSIX
    I/O. That almost certainly isn't worth it.