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.
This commit is contained in:
Willy Tarreau 2019-06-04 16:27:36 +02:00
parent 42a6621d30
commit f37b140b06

View File

@ -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. */