krb5 commit [krb5-1.12]: Fix krb5_read_message handling [CVE-2014-5355]

[Tom Yu]

https://github.com/krb5/krb5/commit/88a0d9cb69d9e9a4442e66e536ac7346e06b0021
commit 88a0d9cb69d9e9a4442e66e536ac7346e06b0021
Author: Greg Hudson <ghudson at mit.edu>
Date:   Tue Dec 9 12:37:44 2014 -0500

    Fix krb5_read_message handling [CVE-2014-5355]
    
    In recvauth_common, do not use strcmp against the data fields of
    krb5_data objects populated by krb5_read_message(), as there is no
    guarantee that they are C strings.  Instead, create an expected
    krb5_data value and use data_eq().
    
    In the sample user-to-user server application, check that the received
    client principal name is null-terminated before using it with printf
    and krb5_parse_name.
    
    CVE-2014-5355:
    
    In MIT krb5, when a server process uses the krb5_recvauth function, an
    unauthenticated remote attacker can cause a NULL dereference by
    sending a zero-byte version string, or a read beyond the end of
    allocated storage by sending a non-null-terminated version string.
    The example user-to-user server application (uuserver) is similarly
    vulnerable to a zero-length or non-null-terminated principal name
    string.
    
    The krb5_recvauth function reads two version strings from the client
    using krb5_read_message(), which produces a krb5_data structure
    containing a length and a pointer to an octet sequence.  krb5_recvauth
    assumes that the data pointer is a valid C string and passes it to
    strcmp() to verify the versions.  If the client sends an empty octet
    sequence, the data pointer will be NULL and strcmp() will dereference
    a NULL pointer, causing the process to crash.  If the client sends a
    non-null-terminated octet sequence, strcmp() will read beyond the end
    of the allocated storage, possibly causing the process to crash.
    
    uuserver similarly uses krb5_read_message() to read a client principal
    name, and then passes it to printf() and krb5_parse_name() without
    verifying that it is a valid C string.
    
    The krb5_recvauth function is used by kpropd and the Kerberized
    versions of the BSD rlogin and rsh daemons.  These daemons are usually
    run out of inetd or in a mode which forks before processing incoming
    connections, so a process crash will generally not result in a
    complete denial of service.
    
    Thanks to Tim Uglow for discovering this issue.
    
    CVSSv2: AV:N/AC:L/Au:N/C:N/I:N/A:P/E:POC/RL:OF/RC:C
    
    [tlyu at mit.edu: CVSS score]
    
    (cherry picked from commit 102bb6ebf20f9174130c85c3b052ae104e5073ec)
    (cherry picked from commit 21e4e653d8258d525f4b6ca87797d42a8bccc282)
    
    ticket: 8180 (new)
    version_fixed: 1.12.4
    status: resolved

 src/appl/user_user/server.c |    4 +++-
 src/lib/krb5/krb/recvauth.c |    9 ++++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/appl/user_user/server.c b/src/appl/user_user/server.c
index dbff68e..b136c72 100644
--- a/src/appl/user_user/server.c
+++ b/src/appl/user_user/server.c
@@ -113,8 +113,10 @@ int main(argc, argv)
     }
 #endif
 
+    /* principal name must be sent null-terminated. */
     retval = krb5_read_message(context, (krb5_pointer) &sock, &pname_data);
-    if (retval) {
+    if (retval || pname_data.length == 0 ||
+        pname_data.data[pname_data.length - 1] != '\0') {
         com_err ("uu-server", retval, "reading pname");
         return 2;
     }
diff --git a/src/lib/krb5/krb/recvauth.c b/src/lib/krb5/krb/recvauth.c
index da836283..5adc6dd 100644
--- a/src/lib/krb5/krb/recvauth.c
+++ b/src/lib/krb5/krb/recvauth.c
@@ -59,6 +59,7 @@ recvauth_common(krb5_context context,
     krb5_rcache           rcache = 0;
     krb5_octet            response;
     krb5_data             null_server;
+    krb5_data             d;
     int                   need_error_free = 0;
     int                   local_rcache = 0, local_authcon = 0;
 
@@ -77,7 +78,8 @@ recvauth_common(krb5_context context,
          */
         if ((retval = krb5_read_message(context, fd, &inbuf)))
             return(retval);
-        if (strcmp(inbuf.data, sendauth_version)) {
+        d = make_data((char *)sendauth_version, strlen(sendauth_version) + 1);
+        if (!data_eq(inbuf, d)) {
             problem = KRB5_SENDAUTH_BADAUTHVERS;
             response = 1;
         }
@@ -93,8 +95,9 @@ recvauth_common(krb5_context context,
      */
     if ((retval = krb5_read_message(context, fd, &inbuf)))
         return(retval);
-    if (appl_version && strcmp(inbuf.data, appl_version)) {
-        if (!problem) {
+    if (appl_version != NULL && !problem) {
+        d = make_data(appl_version, strlen(appl_version) + 1);
+        if (!data_eq(inbuf, d)) {
             problem = KRB5_SENDAUTH_BADAPPLVERS;
             response = 2;
         }