From ac8aeb5485a80a06ac8a52b43b84210564cd7e09 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sat, 24 Aug 2024 18:02:52 +0100 Subject: [PATCH] label quoter pools with names --- src/src/acl.c | 10 ++--- src/src/drtables.c | 3 +- src/src/lookups/ibase.c | 2 +- src/src/lookups/ldap.c | 2 +- src/src/lookups/mysql.c | 2 +- src/src/lookups/nisplus.c | 2 +- src/src/lookups/oracle.c | 2 +- src/src/lookups/pgsql.c | 2 +- src/src/lookups/redis.c | 2 +- src/src/lookups/sqlite.c | 2 +- src/src/search.c | 17 ++++---- src/src/spool_in.c | 2 +- src/src/spool_out.c | 10 ++--- src/src/store.c | 85 ++++++++++++++++++++++++--------------- src/src/store.h | 10 +++-- test/runtest | 2 +- test/stderr/2610 | 8 ++-- test/stderr/2620 | 4 +- 18 files changed, 92 insertions(+), 75 deletions(-) diff --git a/src/src/acl.c b/src/src/acl.c index 3f1ff65cb..4f010c925 100644 --- a/src/src/acl.c +++ b/src/src/acl.c @@ -5015,13 +5015,11 @@ FILE * f = (FILE *)ctx; putc('-', f); if (is_tainted(value)) { - int q = quoter_for_address(value); + const uschar * quoter_name; putc('-', f); - if (is_real_quoter(q)) - { - const lookup_info * li = lookup_with_acq_num(q); - fprintf(f, "(%s)", li ? li->name : US"???"); - } + (void) quoter_for_address(value, "er_name); + if (quoter_name) + fprintf(f, "(%s)", quoter_name); } fprintf(f, "acl%c %s %d\n%s\n", name[0], name+1, Ustrlen(value), value); } diff --git a/src/src/drtables.c b/src/src/drtables.c index d25db887f..0c3dd7049 100644 --- a/src/src/drtables.c +++ b/src/src/drtables.c @@ -250,6 +250,8 @@ for (int j = 0; j < lmi->lookupcount; j++) +/* Hunt for the lookup with the given acquisition number */ + static unsigned hunt_acq; static void @@ -259,7 +261,6 @@ lookup_info * li = (lookup_info *)ptr; if (li->acq_num == hunt_acq) *(lookup_info **)ctx = li; } -/*XXX many of the calls here could instead use a name on the quoted-pool */ const lookup_info * lookup_with_acq_num(unsigned k) { diff --git a/src/src/lookups/ibase.c b/src/src/lookups/ibase.c index 7e4973e01..6125b75b7 100644 --- a/src/src/lookups/ibase.c +++ b/src/src/lookups/ibase.c @@ -511,7 +511,7 @@ if (opt) while ((c = *t++)) if (c == '\'') count++; -t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx); +t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx, US(ibase)); while ((c = *s++)) if (c == '\'') { *t++ = '\''; *t++ = '\''; } diff --git a/src/src/lookups/ldap.c b/src/src/lookups/ldap.c index ee4099419..685a9afb0 100644 --- a/src/src/lookups/ldap.c +++ b/src/src/lookups/ldap.c @@ -1467,7 +1467,7 @@ while ((c = *t++)) /* Get sufficient store to hold the quoted string */ -t = quoted = store_get_quoted(len + count + 1, s, idx); +t = quoted = store_get_quoted(len + count + 1, s, idx, US"ldap"); /* Handle plain quote_ldap */ diff --git a/src/src/lookups/mysql.c b/src/src/lookups/mysql.c index 7e0343233..6b4638f70 100644 --- a/src/src/lookups/mysql.c +++ b/src/src/lookups/mysql.c @@ -433,7 +433,7 @@ while ((c = *t++)) /* Old code: if (count == 0) return s; Now always allocate and copy, to track the quoted status. */ -t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx); +t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx, US"mysql"); while ((c = *s++)) { diff --git a/src/src/lookups/nisplus.c b/src/src/lookups/nisplus.c index 3f89c7f81..7d11c0412 100644 --- a/src/src/lookups/nisplus.c +++ b/src/src/lookups/nisplus.c @@ -242,7 +242,7 @@ if (opt) return NULL; /* No options recognized */ while (*t) if (*t++ == '\"') count++; -t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx); +t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx, US"nisplus"); while (*s) { diff --git a/src/src/lookups/oracle.c b/src/src/lookups/oracle.c index 9eb936b25..61285ea05 100644 --- a/src/src/lookups/oracle.c +++ b/src/src/lookups/oracle.c @@ -560,7 +560,7 @@ if (opt) return NULL; /* No options are recognized */ while ((c = *t++)) if (strchr("\n\t\r\b\'\"\\", c) != NULL) count++; -t = quoted = store_get_quoted((int)Ustrlen(s) + count + 1, s, idx); +t = quoted = store_get_quoted((int)Ustrlen(s) + count + 1, s, idx, US"oracle"); while ((c = *s++)) { diff --git a/src/src/lookups/pgsql.c b/src/src/lookups/pgsql.c index 144663f39..9d97afca9 100644 --- a/src/src/lookups/pgsql.c +++ b/src/src/lookups/pgsql.c @@ -431,7 +431,7 @@ if (opt) return NULL; /* No options recognized */ while ((c = *t++)) if (Ustrchr("\n\t\r\b\'\"\\", c) != NULL) count++; -t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx); +t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx, US"pgsql"); while ((c = *s++)) { diff --git a/src/src/lookups/redis.c b/src/src/lookups/redis.c index a97496ba6..9d3f2d44b 100644 --- a/src/src/lookups/redis.c +++ b/src/src/lookups/redis.c @@ -416,7 +416,7 @@ if (opt) return NULL; /* No options recognized */ while ((c = *t++)) if (isspace(c) || c == '\\') count++; -t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx); +t = quoted = store_get_quoted(Ustrlen(s) + count + 1, s, idx, US"redis"); while ((c = *s++)) { diff --git a/src/src/lookups/sqlite.c b/src/src/lookups/sqlite.c index 9f6c9e5aa..82c4ec12a 100644 --- a/src/src/lookups/sqlite.c +++ b/src/src/lookups/sqlite.c @@ -143,7 +143,7 @@ if (opt) return NULL; /* No options recognized */ while ((c = *t++)) if (c == '\'') count++; count += t - s; -t = quoted = store_get_quoted(count + 1, s, idx); +t = quoted = store_get_quoted(count + 1, s, idx, US"sqlite"); while ((c = *s++)) { diff --git a/src/src/search.c b/src/src/search.c index 5078920f1..efa720f25 100644 --- a/src/src/search.c +++ b/src/src/search.c @@ -514,7 +514,7 @@ search_cache * c = (search_cache *)(t->data.ptr); const lookup_info * li = c->li; expiring_data * e = NULL; /* compiler quietening */ uschar * data = NULL; -int quoter_id = li->acq_num; +int required_quoter_id = li->acq_num; int old_pool = store_pool; /* Lookups that return DEFER may not always set an error message. So that @@ -577,11 +577,11 @@ else XXX Should we this move into lf_sqlperform() ? The server-taint check is there. Also it already knows about looking for a "servers" spec in the query string. - Passing quoter_id down that far is an issue. + Passing required_quoter_id down that far is an issue. */ if ( !filename && li->quote - && is_tainted(keystring) && !is_quoted_like(keystring, quoter_id)) + && is_tainted(keystring) && !is_quoted_like(keystring, required_quoter_id)) { const uschar * ks = keystring; uschar * loc = acl_current_verb(); @@ -613,13 +613,12 @@ else DEBUG(D_lookup) { - int q = quoter_for_address(ks); - const lookup_info * qli = lookup_with_acq_num(q); + const uschar * quoter_name; + int q = quoter_for_address(ks, "er_name); - debug_printf_indent("quoter_id %d (%s) quoting %d (%s)\n", - quoter_id, li->name, - q, - is_real_quoter(q) ? qli ? qli->name : US"???" : US"none"); + debug_printf_indent("required_quoter_id %d (%s) quoting %d (%s)\n", + required_quoter_id, li->name, + q, quoter_name); } #endif } diff --git a/src/src/spool_in.c b/src/src/spool_in.c index 76b2f615b..bb54571be 100644 --- a/src/src/spool_in.c +++ b/src/src/spool_in.c @@ -526,7 +526,7 @@ for (;;) where = NULL; goto SPOOL_FORMAT_ERROR; } - proto_mem = store_get_quoted(1, GET_TAINTED, li->acq_num); + proto_mem = store_get_quoted(1, GET_TAINTED, li->acq_num, li->name); } #endif /* COMPILE_UTILITY */ var = s + 1; diff --git a/src/src/spool_out.c b/src/src/spool_out.c index e1972aa44..78bf48cad 100644 --- a/src/src/spool_out.c +++ b/src/src/spool_out.c @@ -124,13 +124,11 @@ spool_var_write(FILE * fp, const uschar * name, const uschar * val) putc('-', fp); if (is_tainted(val)) { - int q = quoter_for_address(val); + const uschar * quoter_name; putc('-', fp); - if (is_real_quoter(q)) - { - const lookup_info * li = lookup_with_acq_num(q); - fprintf(fp, "(%s)", li ? li->name : US"unknown!"); - } + (void) quoter_for_address(val, "er_name); + if (quoter_name) + fprintf(fp, "(%s)", quoter_name); } fprintf(fp, "%s %s\n", name, val); } diff --git a/src/src/store.c b/src/src/store.c index 09f812dfd..c135b4383 100644 --- a/src/src/store.c +++ b/src/src/store.c @@ -132,6 +132,7 @@ typedef struct pooldesc { typedef struct quoted_pooldesc { pooldesc pool; unsigned quoter; + const unsigned char * quoter_name; struct quoted_pooldesc * next; } quoted_pooldesc; @@ -335,24 +336,27 @@ log_write(0, LOG_MAIN|LOG_PANIC_DIE, "Taint mismatch, %s: %s %d\n", /* Return the pool for the given quoter, or null */ static pooldesc * -pool_for_quoter(unsigned quoter) +pool_for_quoter(unsigned quoter, const uschar ** namep) { for (quoted_pooldesc * qp = quoted_pools; qp; qp = qp->next) if (qp->quoter == quoter) + { + if (namep) *namep = qp->quoter_name; return &qp->pool; + } return NULL; } /* Allocate/init a new quoted-pool and return the pool */ static pooldesc * -quoted_pool_new(unsigned quoter) +quoted_pool_new(unsigned quoter, const uschar * quoter_name) { -// debug_printf("allocating quoted-pool\n"); quoted_pooldesc * qp = store_get_perm(sizeof(quoted_pooldesc), GET_UNTAINTED); pool_init(&qp->pool); qp->quoter = quoter; +qp->quoter_name = quoter_name; qp->next = quoted_pools; quoted_pools = qp; return &qp->pool; @@ -504,13 +508,14 @@ void * store_get_3(int size, const void * proto_mem, const char * func, int linenumber) { #ifndef COMPILE_UTILITY -int quoter = quoter_for_address(proto_mem); +const uschar * quoter_name; +int quoter = quoter_for_address(proto_mem, "er_name); #endif pooldesc * pp; void * yield; #ifndef COMPILE_UTILITY -if (!is_real_quoter(quoter)) +if (!quoter_name) #endif { BOOL tainted = is_tainted(proto_mem); @@ -533,7 +538,8 @@ else DEBUG(D_memory) debug_printf("allocating quoted-block for quoter %u (from %s %d)\n", quoter, func, linenumber); - if (!(pp = pool_for_quoter(quoter))) pp = quoted_pool_new(quoter); + if (!(pp = pool_for_quoter(quoter, NULL))) + pp = quoted_pool_new(quoter, quoter_name); yield = pool_get(pp, size, FALSE, func, linenumber); DEBUG(D_memory) debug_printf("---QQ Get %6p %5d %-14s %4d\n", @@ -592,16 +598,16 @@ Return: allocated memory block */ static void * -store_force_get_quoted(int size, unsigned quoter, +store_force_get_quoted(int size, unsigned quoter, const uschar * quoter_name, const char * func, int linenumber) { -pooldesc * pp = pool_for_quoter(quoter); +pooldesc * pp = pool_for_quoter(quoter, NULL); void * yield; DEBUG(D_memory) debug_printf("allocating quoted-block for quoter %u (from %s %d)\n", quoter, func, linenumber); -if (!pp) pp = quoted_pool_new(quoter); +if (!pp) pp = quoted_pool_new(quoter, quoter_name); yield = pool_get(pp, size, FALSE, func, linenumber); DEBUG(D_memory) @@ -616,32 +622,37 @@ prototype memory is tainted. Otherwise, get plain memory. */ void * store_get_quoted_3(int size, const void * proto_mem, unsigned quoter, - const char * func, int linenumber) + const uschar * quoter_name, const char * func, int linenumber) { -// debug_printf("store_get_quoted_3: quoter %u\n", quoter); return is_tainted(proto_mem) - ? store_force_get_quoted(size, quoter, func, linenumber) + ? store_force_get_quoted(size, quoter, quoter_name, func, linenumber) : store_get_3(size, proto_mem, func, linenumber); } /* Return quoter for given address, or -1 if not in a quoted-pool. */ int -quoter_for_address(const void * p) +quoter_for_address(const void * p, const uschar ** namep) { -for (quoted_pooldesc * qp = quoted_pools; qp; qp = qp->next) +const quoted_pooldesc * qp; +for (qp = quoted_pools; qp; qp = qp->next) { - pooldesc * pp = &qp->pool; + const pooldesc * pp = &qp->pool; storeblock * b; if (b = pp->current_block) if (is_pointer_in_block(b, p)) - return qp->quoter; + goto found; for (b = pp->chainbase; b; b = b->next) if (is_pointer_in_block(b, p)) - return qp->quoter; + goto found; } +if (namep) *namep = NULL; return -1; + +found: + if (namep) *namep = qp->quoter_name; + return qp->quoter; } /* Return TRUE iff the given address is quoted for the given type. @@ -650,12 +661,22 @@ find variants but shared quote functions. */ BOOL is_quoted_like(const void * p, unsigned quoter) { -int pq = quoter_for_address(p); -const lookup_info * p_li = lookup_with_acq_num(pq); -void * p_qfn = p_li ? p_li->quote : NULL; -const lookup_info * q_li = lookup_with_acq_num(quoter); -void * q_qfn = q_li ? q_li->quote : NULL; -BOOL y = is_real_quoter(pq) && p_qfn == q_qfn; +const uschar * p_name, * q_name; +const lookup_info * p_li, * q_li; +void * p_qfn, * q_qfn; + +(void) quoter_for_address(p, &p_name); +(void) pool_for_quoter(quoter, &q_name); + +if (!p_name || !q_name) return FALSE; + +p_li = search_findtype(p_name, Ustrlen(p_name)); +p_qfn = p_li ? p_li->quote : NULL; +q_li = search_findtype(q_name, Ustrlen(q_name)); +q_qfn = q_li ? q_li->quote : NULL; + +BOOL y = p_qfn == q_qfn; + /* debug_printf("is_quoted(%p, %u): %c\n", p, quoter, y?'T':'F'); */ return y; } @@ -667,6 +688,7 @@ is_real_quoter(int quoter) return quoter >= 0; } + /* Return TRUE if the "new" data requires that the "old" data be recopied to new-class memory. We order the classes as @@ -684,8 +706,8 @@ is_incompatible_fn(const void * old, const void * new) int oq, nq; unsigned oi, ni; -ni = is_real_quoter(nq = quoter_for_address(new)) ? 1 : is_tainted(new) ? 2 : 0; -oi = is_real_quoter(oq = quoter_for_address(old)) ? 1 : is_tainted(old) ? 2 : 0; +ni = is_real_quoter(nq = quoter_for_address(new, NULL)) ? 1 : is_tainted(new) ? 2 : 0; +oi = is_real_quoter(oq = quoter_for_address(old, NULL)) ? 1 : is_tainted(old) ? 2 : 0; return ni > oi || ni == oi && nq != oq; } @@ -1257,10 +1279,9 @@ DEBUG(D_memory) for (quoted_pooldesc * qp = quoted_pools; qp; i++, qp = qp->next) { pooldesc * pp = &qp->pool; - const lookup_info* li = lookup_with_acq_num(qp->quoter); debug_printf("----Exit pool Q%d max: %3d kB in %d blocks at order %u\ttainted quoted:%s\n", i, (pp->maxbytes+1023)/1024, pp->maxblocks, pp->maxorder, - li ? li->name : US"???"); + qp->quoter_name); } } #endif @@ -1299,14 +1320,12 @@ store_pool = oldpool; void debug_print_taint(const void * p) { -int q = quoter_for_address(p); +const uschar * quoter_name; if (!is_tainted(p)) return; debug_printf("(tainted"); -if (is_real_quoter(q)) - { - const lookup_info * li = lookup_with_acq_num(q); - debug_printf(", quoted:%s", li ? li->name : US"???"); - } +(void) quoter_for_address(p, "er_name); +if (quoter_name) + debug_printf(", quoted:%s", quoter_name); debug_printf(")\n"); } #endif diff --git a/src/src/store.h b/src/src/store.h index 834457aaa..f088647f1 100644 --- a/src/src/store.h +++ b/src/src/store.h @@ -50,8 +50,9 @@ tracing information for debugging. */ #define store_free(addr) \ store_free_3(addr, __FUNCTION__, __LINE__) /* store_get & store_get_perm are in local_scan.h */ -#define store_get_quoted(size, proto_mem, quoter) \ - store_get_quoted_3((size), (proto_mem), (quoter), __FUNCTION__, __LINE__) +#define store_get_quoted(size, proto_mem, quoter, quoter_name) \ + store_get_quoted_3((size), (proto_mem), (quoter), (quoter_name), \ + __FUNCTION__, __LINE__) #define store_malloc(size) \ store_malloc_3(size, __FUNCTION__, __LINE__) #define store_mark(void) \ @@ -70,7 +71,8 @@ typedef void ** rmark; extern BOOL store_extend_3(void *, int, int, const char *, int); extern void store_free_3(void *, const char *, int); /* store_get_3 & store_get_perm_3 are in local_scan.h */ -extern void * store_get_quoted_3(int, const void *, unsigned, const char *, int); +extern void * store_get_quoted_3(int, const void *, unsigned, const uschar *, + const char *, int); extern void * store_malloc_3(size_t, const char *, int) ALLOC ALLOC_SIZE(1) WARN_UNUSED_RESULT; extern rmark store_mark_3(const char *, int); extern void * store_newblock_3(void *, int, int, const char *, int); @@ -80,7 +82,7 @@ extern rmark store_reset_3(rmark, const char *, int); #define GET_UNTAINTED (const void *)0 #define GET_TAINTED (const void *)1 -extern int quoter_for_address(const void *); +extern int quoter_for_address(const void *, const uschar **); extern BOOL is_quoted_like(const void *, unsigned); extern BOOL is_real_quoter(int); extern void debug_print_taint(const void * p); diff --git a/test/runtest b/test/runtest index ab8371942..e80624484 100755 --- a/test/runtest +++ b/test/runtest @@ -1471,7 +1471,7 @@ RESET_AFTER_EXTRA_LINE_READ: } # Different builds will have different lookup types included - s/quoter_id \K\d+ \((\w+)\) quoting -1 \(none\)$/NN ($1) quoting -1 (none)/; + s/required_quoter_id \K\d+ \((\w+)\) quoting -1 \(NULL\)$/NN ($1) quoting -1 (NULL)/; # and different numbers of lookup types result in different type-code letters, # so convert them all to "0" s%(?