Date: Mon, 21 Oct 2013 07:08:39 +0200 From: Dirk Engling <erdgeist@erdgeist.org> To: freebsd-jail@FreeBSD.org Subject: BUG in jail(8) variable substitution, and PATCH Message-ID: <5264B6D7.7070901@erdgeist.org>
next in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------030405060009080902080007
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
The variable substitution of FreeBSD's jail tool yields unexpected
results when a parameter has more than one variable to substitute and
one of the later variables needs substitution as well.
Consider the simple test case:
$A = "A_${B}_C_${D}";
$B = "BBBBB";
$D = "DDDDD_${E}_FFFFF";
$E = "EEEEE";
bar {
exec.poststart = "touch /tmp/$A";
}
EXPECTED OUTCOME for running "jail -c bar" would be a file with the name
/tmp/A_BBBBB_C_DDDDD_EEEEE_FFFFF to be touched (and, of course, the jail
bar being created).
OBSERVED OUTCOME is a file with the name
/tmp/A_BBBDDDDD_EEEEE_FFFFFBB_C_ being created.
The reason is the way jail(8) resolves recursive substitutions. In
head/usr.sbin/jail/config.c:193 a varoff variable is introduced that
handles a shifting offset for multiple variable substitutions per
parameter. This varoff is updated after each substitution in line 239 to
reflect a new offset into the parameter's string. This ensures that all
other variables are substituted at [their insertion point plus varoff]
which is the accumulated length of all previously substituted variables.
Now in our example, if $A is to be expanded, first ${B} is inserted at
offset 2 and varoff becomes 10. When substituting ${D}, the recursion
check at line 216 detects that variable $D also needs expansion. It
reorders the parameter list, so that the algorithm works on variable $D
now. Then it jumps to find_vars at line 191 and properly expands
DDDDD_${E}_FFFFF to DDDDD_EEEEE_FFFFF.
When the algorithm now returns to expanding $A by entering the loop body
again, it finds a re-set varoff variable leading to (the now expanded)
variable $D being inserted at the offset 5, where the parser initially
would find it (the internal format for $A is approx: { "A__C_", {2,
"B"}, {5, "D"}}) and not at the corrected offset 10.
PROPOSED SOLUTION: Get rid of the varoff and replace line 239 with:
[struct cfvar *]vv = v;
while ((vv = STAILQ_NEXT(vv, tq)))
v->pos += vs->len;
to make the offset permanent. Find a patch attached.
Regards,
erdgeist
--------------030405060009080902080007
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
name="config.c.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="config.c.patch"
Index: config.c
===================================================================
--- config.c (revision 256751)
+++ config.c (working copy)
@@ -129,9 +129,8 @@
struct cfjail *j, *tj, *wj;
struct cfparam *p, *vp, *tp;
struct cfstring *s, *vs, *ns;
- struct cfvar *v;
+ struct cfvar *v, *vv;
char *ep;
- size_t varoff;
int did_self, jseq, pgen;
if (!strcmp(cfname, "-")) {
@@ -190,7 +189,6 @@
p->gen = ++pgen;
find_vars:
TAILQ_FOREACH(s, &p->val, tq) {
- varoff = 0;
while ((v = STAILQ_FIRST(&s->vars))) {
TAILQ_FOREACH(vp, &j->params, tq)
if (!strcmp(vp->name, v->name))
@@ -232,11 +230,13 @@
goto bad_var;
}
s->s = erealloc(s->s, s->len + vs->len + 1);
- memmove(s->s + v->pos + varoff + vs->len,
- s->s + v->pos + varoff,
- s->len - (v->pos + varoff) + 1);
- memcpy(s->s + v->pos + varoff, vs->s, vs->len);
- varoff += vs->len;
+ memmove(s->s + v->pos + vs->len,
+ s->s + v->pos,
+ s->len - v->pos + 1);
+ memcpy(s->s + v->pos, vs->s, vs->len);
+ vv = v;
+ while ((vv = STAILQ_NEXT(vv, tq)))
+ vv->pos += vs->len;
s->len += vs->len;
while ((vs = TAILQ_NEXT(vs, tq))) {
ns = emalloc(sizeof(struct cfstring));
--------------030405060009080902080007--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5264B6D7.7070901>