From b7719af4ce027f686f8ce10c45fc2f62e073ec3c Mon Sep 17 00:00:00 2001
From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sat, 10 Nov 2012 19:03:28 +1300
Subject: [PATCH] Safer use of strwordtok()

* It does not allocate. Drop a bad free()

* It may return NULL. Check for that before URL-decodeing,
  and also handle TT result mandatory token field better.

Caught in audit by Alex Rousskov.
---
 src/HelperReply.cc | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/HelperReply.cc b/src/HelperReply.cc
index 06531cadf1..de2f29d134 100644
--- a/src/HelperReply.cc
+++ b/src/HelperReply.cc
@@ -50,10 +50,16 @@ HelperReply::parse(char *buf, size_t len, bool urlQuoting)
             p+=3;
             // followed by an auth token
             char *w1 = strwordtok(NULL, &p);
-            MemBuf authToken;
-            authToken.init();
-            authToken.append(w1, strlen(w1));
-            responseKeys.add("token",authToken.content());
+            if (w1 != NULL) {
+                MemBuf authToken;
+                authToken.init();
+                authToken.append(w1, strlen(w1));
+                responseKeys.add("token",authToken.content());
+            } else {
+                // token field is mandatory on this response code
+                result = HelperReply::BrokenHelper;
+                responseKeys.add("message","Missing 'token' data");
+            }
 
         } else if (!strncmp(p,"AF ",3)) {
             // NTLM/Negotate OK response
@@ -126,10 +132,9 @@ HelperReply::parseResponseKeys(bool urlQuotingValues)
         // the value may be a quoted string or a token
         // XXX: eww. update strwordtok() to be zero-copy
         char *v = strwordtok(NULL, &p);
-        if ((p-v) > 2) // 1-octet %-escaped requires 3 bytes
+        if (v != NULL && (p-v) > 2) // 1-octet %-escaped requires 3 bytes
             rfc1738_unescape(v);
         String value = v;
-        safe_free(v);
 
         responseKeys.add(key, value);
 
-- 
2.47.3