From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Fri, 1 Sep 2023 10:44:32 +0000 (+0100)
Subject: Taint: track SASL auth intermediate inputs
X-Git-Tag: exim-4.97-RC0~15
X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/9b810c775c6e9dd1f8a87a743b943b465a1ca5a1

Taint: track SASL auth intermediate inputs
---

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index f2802d2fb..b1b79c240 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -185,6 +185,11 @@ JH/37 Bug 3016: Avoid sending DSN when message was accepted under fakereject
       or fakedefer.  Previously the sender could discover that the message
       had in fact been accepted.
 
+JH/38 Taint-track intermediate values from the peer in multi-stage authentation
+      sequences.  Previously the input was not noted as being tainted; notably
+      this resulted in behaviour of LOGIN vs. PLAIN being inconsistent under
+      bad coding of authenticators.
+
 
 Exim version 4.96
 -----------------
diff --git a/src/src/auths/cram_md5.c b/src/src/auths/cram_md5.c
index 280b5293a..583080211 100644
--- a/src/src/auths/cram_md5.c
+++ b/src/src/auths/cram_md5.c
@@ -163,13 +163,13 @@ md5_end(&base, md5secret, 16, digestptr);
 /* For interface, see auths/README */
 
 int
-auth_cram_md5_server(auth_instance *ablock, uschar *data)
+auth_cram_md5_server(auth_instance * ablock, uschar * data)
 {
-auth_cram_md5_options_block *ob =
+auth_cram_md5_options_block * ob =
   (auth_cram_md5_options_block *)(ablock->options_block);
-uschar *challenge = string_sprintf("<%d.%ld@%s>", getpid(),
+uschar * challenge = string_sprintf("<%d.%ld@%s>", getpid(),
     (long int) time(NULL), primary_hostname);
-uschar *clear, *secret;
+uschar * clear, * secret;
 uschar digest[16];
 int i, rc, len;
 
@@ -186,7 +186,7 @@ if (*data) return UNEXPECTED;
 /* Send the challenge, read the return */
 
 if ((rc = auth_get_data(&data, challenge, Ustrlen(challenge))) != OK) return rc;
-if ((len = b64decode(data, &clear)) < 0) return BAD64;
+if ((len = b64decode(data, &clear, GET_TAINTED)) < 0) return BAD64;
 
 /* The return consists of a user name, space-separated from the CRAM-MD5
 digest, expressed in hex. Extract the user name and put it in $auth1 and $1.
@@ -298,7 +298,7 @@ if (smtp_write_command(sx, SCMD_FLUSH, "AUTH %s\r\n", ablock->public_name) < 0)
 if (!smtp_read_response(sx, buffer, buffsize, '3', timeout))
   return FAIL;
 
-if (b64decode(buffer + 4, &challenge) < 0)
+if (b64decode(buffer + 4, &challenge, buffer + 4) < 0)
   {
   string_format(buffer, buffsize, "bad base 64 string in challenge: %s",
     big_buffer + 4);
diff --git a/src/src/auths/cyrus_sasl.c b/src/src/auths/cyrus_sasl.c
index b5d2d1d3b..a3d3906b8 100644
--- a/src/src/auths/cyrus_sasl.c
+++ b/src/src/auths/cyrus_sasl.c
@@ -204,16 +204,16 @@ sasl_done();
 within a shortlived child */
 
 int
-auth_cyrus_sasl_server(auth_instance *ablock, uschar *data)
+auth_cyrus_sasl_server(auth_instance * ablock, uschar * data)
 {
-auth_cyrus_sasl_options_block *ob =
+auth_cyrus_sasl_options_block * ob =
   (auth_cyrus_sasl_options_block *)(ablock->options_block);
-uschar *output, *out2, *input, *clear, *hname;
-uschar *debug = NULL;   /* Stops compiler complaining */
+uschar * output, * out2, * input, * clear, * hname;
+uschar * debug = NULL;   /* Stops compiler complaining */
 sasl_callback_t cbs[] = {{SASL_CB_LIST_END, NULL, NULL}};
-sasl_conn_t *conn;
+sasl_conn_t * conn;
 char * realm_expanded = NULL;
-int rc, firsttime = 1, clen, *negotiated_ssf_ptr = NULL, negotiated_ssf;
+int rc, firsttime = 1, clen, * negotiated_ssf_ptr = NULL, negotiated_ssf;
 unsigned int inlen, outlen;
 
 input = data;
@@ -232,7 +232,7 @@ if (!hname  ||  !realm_expanded  && ob->server_realm)
 
 if (inlen)
   {
-  if ((clen = b64decode(input, &clear)) < 0)
+  if ((clen = b64decode(input, &clear, input)) < 0)
     return BAD64;
   input = clear;
   inlen = clen;
@@ -345,10 +345,10 @@ for (rc = SASL_CONTINUE; rc == SASL_CONTINUE; )
       }
     inlen = Ustrlen(input);
 
-    HDEBUG(D_auth) debug = string_copy(input);
+    HDEBUG(D_auth) debug = string_copy_taint(input, GET_TAINTED);
     if (inlen)
       {
-      if ((clen = b64decode(input, &clear)) < 0)
+      if ((clen = b64decode(input, &clear, GET_TAINTED)) < 0)
        {
        sasl_dispose(&conn);
        sasl_done();
diff --git a/src/src/auths/get_data.c b/src/src/auths/get_data.c
index caf4cfdb8..4a35ed064 100644
--- a/src/src/auths/get_data.c
+++ b/src/src/auths/get_data.c
@@ -33,7 +33,7 @@ else
   uschar * clear, * end;
   int len;
 
-  if ((len = b64decode(data, &clear)) < 0) return BAD64;
+  if ((len = b64decode(data, &clear, GET_TAINTED)) < 0) return BAD64;
   DEBUG(D_auth) debug_printf("auth input decode:");
   for (end = clear + len; clear < end && expand_nmax < EXPAND_MAXN; )
     {
@@ -66,6 +66,10 @@ Arguments:
 Returns:      OK on success
               BAD64 if response too large for buffer
               CANCELLED if response is "*"
+
+NOTE: the data came from the wire so should be tainted - but
+big_buffer is not taint-tracked.  EVERY CALLER needs to apply
+tainting.
 */
 
 int
@@ -97,7 +101,7 @@ uschar * resp, * clear, * end;
 
 if ((rc = auth_get_data(&resp, challenge, Ustrlen(challenge))) != OK)
   return rc;
-if ((len = b64decode(resp, &clear)) < 0)
+if ((len = b64decode(resp, &clear, GET_TAINTED)) < 0)
   return BAD64;
 end = clear + len;
 
@@ -228,7 +232,7 @@ if (flags & AUTH_ITEM_LAST)
 /* Now that we know we'll continue, we put the received data into $auth<n>,
 if possible. First, decode it: buffer+4 skips over the SMTP status code. */
 
-clear_len = b64decode(buffer+4, &clear);
+clear_len = b64decode(buffer+4, &clear, buffer+4);
 
 /* If decoding failed, the default is to terminate the authentication, and
 return FAIL, with the SMTP response still in the buffer. However, if client_
diff --git a/src/src/auths/heimdal_gssapi.c b/src/src/auths/heimdal_gssapi.c
index 7a74d5be5..59884ef58 100644
--- a/src/src/auths/heimdal_gssapi.c
+++ b/src/src/auths/heimdal_gssapi.c
@@ -334,7 +334,7 @@ while (step < 4)
       break;
 
     case 1:
-      gbufdesc_in.length = b64decode(from_client, USS &gbufdesc_in.value);
+      gbufdesc_in.length = b64decode(from_client, USS &gbufdesc_in.value, GET_TAINTED);
       if (gclient)
         {
 	maj_stat = gss_release_name(&min_stat, &gclient);
@@ -419,7 +419,7 @@ while (step < 4)
       break;
 
     case 3:
-      gbufdesc_in.length = b64decode(from_client, USS &gbufdesc_in.value);
+      gbufdesc_in.length = b64decode(from_client, USS &gbufdesc_in.value, GET_TAINTED);
       maj_stat = gss_unwrap(&min_stat,
 	  gcontext,
 	  &gbufdesc_in,       /* data from client */
diff --git a/src/src/base64.c b/src/src/base64.c
index e9ac41a55..591ea3d5b 100644
--- a/src/src/base64.c
+++ b/src/src/base64.c
@@ -152,7 +152,7 @@ static uschar dec64table[] = {
 };
 
 int
-b64decode(const uschar *code, uschar **ptr)
+b64decode(const uschar * code, uschar ** ptr, const void * proto_mem)
 {
 
 int x, y;
@@ -160,7 +160,7 @@ uschar *result;
 
  {
   int l = Ustrlen(code);
-  *ptr = result = store_get(1 + l/4 * 3 + l%4, code);
+  *ptr = result = store_get(1 + l/4 * 3 + l%4, proto_mem);
  }
 
 /* Each cycle of the loop handles a quantum of 4 input bytes. For the last
diff --git a/src/src/expand.c b/src/src/expand.c
index 590b40383..b4a76b3e7 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -8135,7 +8135,7 @@ NOT_ITEM: ;
 	case EOP_BASE64D:
 	  {
 	  uschar * s;
-	  int len = b64decode(sub, &s);
+	  int len = b64decode(sub, &s, sub);
 	  if (len < 0)
 	    {
 	    expand_string_message = string_sprintf("string \"%s\" is not "
diff --git a/src/src/functions.h b/src/src/functions.h
index 5db9bc610..4222c623a 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -157,7 +157,7 @@ extern gstring *authres_spf(gstring *);
 
 extern uschar *b64encode(const uschar *, int);
 extern uschar *b64encode_taint(const uschar *, int, const void *);
-extern int     b64decode(const uschar *, uschar **);
+extern int     b64decode(const uschar *, uschar **, const void *);
 extern int     bdat_getc(unsigned);
 extern uschar *bdat_getbuf(unsigned *);
 extern BOOL    bdat_hasc(void);
diff --git a/src/src/lss.c b/src/src/lss.c
index e6ec1d6d1..55df5775e 100644
--- a/src/src/lss.c
+++ b/src/src/lss.c
@@ -134,9 +134,9 @@ A zero is added on to the end to make it easy in cases where the result is to
 be interpreted as text. This is not included in the count. */
 
 int
-lss_b64decode(uschar *code, uschar **ptr)
+lss_b64decode(uschar * code, uschar ** ptr)
 {
-return b64decode(code, ptr);
+return b64decode(code, ptr, code);
 }
 
 
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index c8f180a58..30cb0437c 100644
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -448,7 +448,7 @@ return n;
 void
 pdkim_decode_base64(const uschar * str, blob * b)
 {
-int dlen = b64decode(str, &b->data);
+int dlen = b64decode(str, &b->data, str);
 if (dlen < 0) b->data = NULL;
 b->len = dlen;
 }
diff --git a/src/src/pdkim/signing.c b/src/src/pdkim/signing.c
index 07737ab41..35ca79fc1 100644
--- a/src/src/pdkim/signing.c
+++ b/src/src/pdkim/signing.c
@@ -419,7 +419,7 @@ if (  !(s1 = Ustrstr(CS privkey_pem, "-----BEGIN RSA PRIVATE KEY-----"))
 
 *s2 = '\0';
 
-if ((rc = b64decode(s1, &der.data) < 0))
+if ((rc = b64decode(s1, &der.data, s1) < 0))
   return US"Bad PEM-DER b64 decode";
 der.len = rc;
 
diff --git a/src/src/rfc2047.c b/src/src/rfc2047.c
index d5e33b9b1..9d7a6e023 100644
--- a/src/src/rfc2047.c
+++ b/src/src/rfc2047.c
@@ -122,7 +122,7 @@ for (;; string = mimeword + 2)
   encoding = toupper((*q1ptr)[1]);
   **endptr = 0;
   if (encoding == 'B')
-    dlen = b64decode(*q2ptr+1, dptrptr);
+    dlen = b64decode(*q2ptr+1, dptrptr, *q2ptr+1);
   else if (encoding == 'Q')
     dlen = rfc2047_qpdecode(*q2ptr+1, dptrptr);
   **endptr = '?';   /* restore */