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 ae7e082..1d83c18 100644
--- a/src/vars.c
+++ b/src/vars.c
@@ -36,6 +36,25 @@
__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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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. */