From 9b810c775c6e9dd1f8a87a743b943b465a1ca5a1 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Fri, 1 Sep 2023 11:44:32 +0100 Subject: [PATCH] Taint: track SASL auth intermediate inputs --- doc/doc-txt/ChangeLog | 5 +++++ src/src/auths/cram_md5.c | 12 ++++++------ src/src/auths/cyrus_sasl.c | 18 +++++++++--------- src/src/auths/get_data.c | 10 +++++++--- src/src/auths/heimdal_gssapi.c | 4 ++-- src/src/base64.c | 4 ++-- src/src/expand.c | 2 +- src/src/functions.h | 2 +- src/src/lss.c | 4 ++-- src/src/pdkim/pdkim.c | 2 +- src/src/pdkim/signing.c | 2 +- src/src/rfc2047.c | 2 +- 12 files changed, 38 insertions(+), 29 deletions(-) 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, 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 */ -- 2.30.2