Bug 189139 - [patch] fix bug in jail(8) variable substitution
Summary: [patch] fix bug in jail(8) variable substitution
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: Hiroki Sato
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-30 20:00 UTC by erdgeist
Modified: 2016-01-25 21:10 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description erdgeist 2014-04-30 20:00:00 UTC
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.

Fix: 

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 here https://erdgeist.org/arts/software/jail/usr.sbin.jail-substitution.patch
How-To-Repeat: 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.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2014-05-04 03:54:32 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-jail

Over to maintainer(s).
Comment 2 Hiroki Sato freebsd_committer freebsd_triage 2015-07-07 18:22:27 UTC
Take.
Comment 3 Hiroki Sato freebsd_committer freebsd_triage 2015-07-07 18:48:19 UTC
I think the proposed fix in this PR is correct and jail(8) still has this issue.  The patch has been submitted as D3018 for review.

https://reviews.freebsd.org/D3018
Comment 4 commit-hook freebsd_committer freebsd_triage 2015-07-08 00:52:11 UTC
A commit references this bug:

Author: hrs
Date: Wed Jul  8 00:51:54 UTC 2015
New revision: 285261
URL: https://svnweb.freebsd.org/changeset/base/285261

Log:
  Fix offset calculation in variable substitution
  in jail.conf.  The following did not work correctly:

   A="A_${B}_C_${D}"
   B="BBBBB"
   D="DDDD_${E}_FFFFF"
   E="EEEEE"

  PR:		189139
  Reviewed by:	jamie
  Differential Revision:	https://reviews.freebsd.org/D3018

Changes:
  head/usr.sbin/jail/config.c