More robust handling of GSSAPI error strings

RFC 2744 does not require string types to be NULL terminated, because their length is explicitly passed as part of the gss_buffer_desc (see Section 3.2.). As such, when printing error strings, the length should be explicitly stated. This is shown in the example code of gss_display_status in Section 5.11. While in practice this is the case (at least from checking MIT and Heimdal's error handling code), it doesn't hurt to be defensive.

In addition, there are some conditions where value may be NULL or length may be 0, so make sure to check for these prior to calling StringPrintf, so as not to crash. Finally, for the extreme defensive case, make sure that the length (which is a size_t) is capped at INT_MAX prior to printing.

Contributed by ryan.sleevi@gmail.com

BUG=33033
TEST=None

Review URL: http://codereview.chromium.org/2646004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49299 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 20 insertions, 6 deletions

diff --git a/net/http/http_auth_gssapi_posix.cc b/net/http/http_auth_gssapi_posix.cc
index 7b6faa1..d49ebe4 100644
--- a/net/http/http_auth_gssapi_posix.cc
+++ b/net/http/http_auth_gssapi_posix.cc
@@ -4,6 +4,8 @@
 
 #include "net/http/http_auth_gssapi_posix.h"
 
+#include <limits>
+
 #include "base/base64.h"
 #include "base/file_path.h"
 #include "base/logging.h"
@@ -208,20 +210,32 @@ std::string DisplayCode(GSSAPILibrary* gssapi_lib,
                         gssapi::OM_uint32 status,
                         gssapi::OM_uint32 status_code_type) {
   const int kMaxDisplayIterations = 8;
+  const size_t kMaxMsgLength = 4096;
   // msg_ctx needs to be outside the loop because it is invoked multiple times.
   gssapi::OM_uint32 msg_ctx = 0;
   std::string rv = StringPrintf("(0x%08X)", status);
 
   // This loop should continue iterating until msg_ctx is 0 after the first
   // iteration. To be cautious and prevent an infinite loop, it stops after
-  // a finite number of iterations as well.
-  for (int i = 0; i < kMaxDisplayIterations; ++i) {
+  // a finite number of iterations as well. As an added sanity check, no
+  // individual message may exceed |kMaxMsgLength|, and the final result
+  // will not exceed |kMaxMsgLength|*2-1.
+  for (int i = 0; i < kMaxDisplayIterations && rv.size() < kMaxMsgLength;
+      ++i) {
     gssapi::OM_uint32 min_stat;
     gssapi::gss_buffer_desc_struct msg = GSS_C_EMPTY_BUFFER;
-    gssapi_lib->display_status(&min_stat, status, status_code_type,
-                               GSS_C_NULL_OID,
-                               &msg_ctx, &msg);
-    rv += StringPrintf(" %s", static_cast<char *>(msg.value));
+    gssapi::OM_uint32 maj_stat =
+        gssapi_lib->display_status(&min_stat, status, status_code_type,
+                                   GSS_C_NULL_OID, &msg_ctx, &msg);
+    if (maj_stat == GSS_S_COMPLETE) {
+      int msg_len = (msg.length > kMaxMsgLength) ?
+          static_cast<int>(kMaxMsgLength) :
+          static_cast<int>(msg.length);
+      if (msg_len > 0 && msg.value != NULL) {
+        rv += StringPrintf(" %.*s", msg_len,
+                           static_cast<char *>(msg.value));
+      }
+    }
     gssapi_lib->release_buffer(&min_stat, &msg);
     if (!msg_ctx)
       break;