From: Willy Tarreau Date: Tue, 4 Jun 2019 14:27:36 +0000 (+0200) Subject: BUG/MEDIUM: vars: make sure the scope is always valid when accessing vars X-Git-Tag: v2.0-dev6~63 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f37b140b06b9963dea8adaf5e13b5b57cd219c74;p=thirdparty%2Fhaproxy.git BUG/MEDIUM: vars: make sure the scope is always valid when accessing vars Patrick Hemmer reported that a simple tcp rule involving a variable like this is enough to crash haproxy : frontend foo bind :8001 tcp-request session set-var(txn.foo) src The tests on the variables scopes is not strict enough, it needs to always verify if the stream is valid when accessing a req/res/txn variable. This patch does this by adding a new get_vars() function which does the job instead of open-coding all the lookups everywhere. It must be backported to all versions supporting set-var and "tcp-request session" so at least 1.9 and 1.8. --- diff --git a/src/vars.c b/src/vars.c index ae7e082d0f..1d83c186aa 100644 --- a/src/vars.c +++ b/src/vars.c @@ -36,6 +36,25 @@ static unsigned int var_reqres_limit = 0; __decl_rwlock(var_names_rwlock); +/* returns the struct vars pointer for a session, stream and scope, or NULL if + * it does not exist. + */ +static inline struct vars *get_vars(struct session *sess, struct stream *strm, enum vars_scope scope) +{ + switch (scope) { + case SCOPE_PROC: + return &global.vars; + case SCOPE_SESS: + return &sess->vars; + case SCOPE_TXN: + return strm ? &strm->vars_txn : NULL; + case SCOPE_REQ: + case SCOPE_RES: + default: + return strm ? &strm->vars_reqres : NULL; + } +} + /* This function adds or remove memory size from the accounting. The inner * pointers may be null when setting the outer ones only. */ @@ -44,10 +63,12 @@ static void var_accounting_diff(struct vars *vars, struct session *sess, struct switch (vars->scope) { case SCOPE_REQ: case SCOPE_RES: - _HA_ATOMIC_ADD(&strm->vars_reqres.size, size); + if (strm) + _HA_ATOMIC_ADD(&strm->vars_reqres.size, size); /* fall through */ case SCOPE_TXN: - _HA_ATOMIC_ADD(&strm->vars_txn.size, size); + if (strm) + _HA_ATOMIC_ADD(&strm->vars_txn.size, size); /* fall through */ case SCOPE_SESS: _HA_ATOMIC_ADD(&sess->vars.size, size); @@ -70,11 +91,11 @@ static int var_accounting_add(struct vars *vars, struct session *sess, struct st switch (vars->scope) { case SCOPE_REQ: case SCOPE_RES: - if (var_reqres_limit && strm->vars_reqres.size + size > var_reqres_limit) + if (var_reqres_limit && strm && strm->vars_reqres.size + size > var_reqres_limit) return 0; /* fall through */ case SCOPE_TXN: - if (var_txn_limit && strm->vars_txn.size + size > var_txn_limit) + if (var_txn_limit && strm && strm->vars_txn.size + size > var_txn_limit) return 0; /* fall through */ case SCOPE_SESS: @@ -287,27 +308,8 @@ static int smp_fetch_var(const struct arg *args, struct sample *smp, const char struct vars *vars; /* Check the availibity of the variable. */ - switch (var_desc->scope) { - case SCOPE_PROC: - vars = &global.vars; - break; - case SCOPE_SESS: - vars = &smp->sess->vars; - break; - case SCOPE_TXN: - if (!smp->strm) - return 0; - vars = &smp->strm->vars_txn; - break; - case SCOPE_REQ: - case SCOPE_RES: - default: - if (!smp->strm) - return 0; - vars = &smp->strm->vars_reqres; - break; - } - if (vars->scope != var_desc->scope) + vars = get_vars(smp->sess, smp->strm, var_desc->scope); + if (!vars || vars->scope != var_desc->scope) return 0; HA_RWLOCK_RDLOCK(VARS_LOCK, &vars->rwlock); @@ -431,15 +433,8 @@ static inline int sample_store_stream(const char *name, enum vars_scope scope, s struct vars *vars; int ret; - switch (scope) { - case SCOPE_PROC: vars = &global.vars; break; - case SCOPE_SESS: vars = &smp->sess->vars; break; - case SCOPE_TXN: vars = &smp->strm->vars_txn; break; - case SCOPE_REQ: - case SCOPE_RES: - default: vars = &smp->strm->vars_reqres; break; - } - if (vars->scope != scope) + vars = get_vars(smp->sess, smp->strm, scope); + if (!vars || vars->scope != scope) return 0; HA_RWLOCK_WRLOCK(VARS_LOCK, &vars->rwlock); @@ -455,15 +450,8 @@ static inline int sample_clear_stream(const char *name, enum vars_scope scope, s struct var *var; unsigned int size = 0; - switch (scope) { - case SCOPE_PROC: vars = &global.vars; break; - case SCOPE_SESS: vars = &smp->sess->vars; break; - case SCOPE_TXN: vars = &smp->strm->vars_txn; break; - case SCOPE_REQ: - case SCOPE_RES: - default: vars = &smp->strm->vars_reqres; break; - } - if (vars->scope != scope) + vars = get_vars(smp->sess, smp->strm, scope); + if (!vars || vars->scope != scope) return 0; /* Look for existing variable name. */ @@ -599,17 +587,8 @@ int vars_get_by_name(const char *name, size_t len, struct sample *smp) return 0; /* Select "vars" pool according with the scope. */ - switch (scope) { - case SCOPE_PROC: vars = &global.vars; break; - case SCOPE_SESS: vars = &smp->sess->vars; break; - case SCOPE_TXN: vars = &smp->strm->vars_txn; break; - case SCOPE_REQ: - case SCOPE_RES: - default: vars = &smp->strm->vars_reqres; break; - } - - /* Check if the scope is available a this point of processing. */ - if (vars->scope != scope) + vars = get_vars(smp->sess, smp->strm, scope); + if (!vars || vars->scope != scope) return 0; /* Get the variable entry. */ @@ -633,17 +612,10 @@ int vars_get_by_desc(const struct var_desc *var_desc, struct sample *smp) struct var *var; /* Select "vars" pool according with the scope. */ - switch (var_desc->scope) { - case SCOPE_PROC: vars = &global.vars; break; - case SCOPE_SESS: vars = &smp->sess->vars; break; - case SCOPE_TXN: vars = &smp->strm->vars_txn; break; - case SCOPE_REQ: - case SCOPE_RES: - default: vars = &smp->strm->vars_reqres; break; - } + vars = get_vars(smp->sess, smp->strm, var_desc->scope); /* Check if the scope is available a this point of processing. */ - if (vars->scope != var_desc->scope) + if (!vars || vars->scope != var_desc->scope) return 0; /* Get the variable entry. */