Taint: track SASL auth intermediate inputs
authorJeremy Harris <jgh146exb@wizmail.org>
Fri, 1 Sep 2023 10:44:32 +0000 (11:44 +0100)
committerJeremy Harris <jgh146exb@wizmail.org>
Fri, 1 Sep 2023 10:44:32 +0000 (11:44 +0100)
12 files changed:
doc/doc-txt/ChangeLog
src/src/auths/cram_md5.c
src/src/auths/cyrus_sasl.c
src/src/auths/get_data.c
src/src/auths/heimdal_gssapi.c
src/src/base64.c
src/src/expand.c
src/src/functions.h
src/src/lss.c
src/src/pdkim/pdkim.c
src/src/pdkim/signing.c
src/src/rfc2047.c

index f2802d2fb87e7f696348532a3a91a1ca3e13d130..b1b79c2403184288358f3ccc5f1d62d6ea6d81cb 100644 (file)
@@ -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
 -----------------
index 280b5293a2e77d64a8215b13f18011ec88b04977..583080211af0f30099836253cc265db0c3efa466 100644 (file)
@@ -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);
index b5d2d1d3b71a08b4967078e7a42a48f955693a9a..a3d3906b885b4c7cbc66f75f40a7753141ed9df5 100644 (file)
@@ -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();
index caf4cfdb8faa79925ad1c3a3ffc7ee18c2f06a4e..4a35ed0649aa22e88001ee9b1df3e900dd9d5cc6 100644 (file)
@@ -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_
index 7a74d5be57f07dbe6d254cc732bcc5aab26f3557..59884ef58e2c32d0f3c2f1567c5dff23f436efe1 100644 (file)
@@ -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 */
index e9ac41a556a42477acdc48b482dbb51b002407bb..591ea3d5b1b15221f126c9f4d103e4d8d2319a40 100644 (file)
@@ -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
index 590b40383dbbe41254df3822ed2fa1d3545a9c58..b4a76b3e7649f9573fcb720cd4a6ed711a6652a3 100644 (file)
@@ -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 "
index 5db9bc610da1ac75586c4776755c6452188f6b17..4222c623a3f77e537fd21f2e2141390ec87780eb 100644 (file)
@@ -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);
index e6ec1d6d13242ea70f40053264d433b495ab0328..55df5775e391a17e948a4ecce1bac3872b7b15cf 100644 (file)
@@ -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);
 }
 
 
index c8f180a588c66ef8fd0fd21e23322a3ee51d75f6..30cb0437c705d86bd4a517b0b6abb00298a83fc1 100644 (file)
@@ -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;
 }
index 07737ab4115c699e087fc8b6f145efc473b95bf7..35ca79fc199b889426755d374905cc341305bf87 100644 (file)
@@ -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;
 
index d5e33b9b1a6ec127e18789dbb85b87e21971e603..9d7a6e02324f039e81aec2355b988b373b97f553 100644 (file)
@@ -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 */