Bug 248444 - /usr/sbin/jail crashes when parsing certain configuration files
Summary: /usr/sbin/jail crashes when parsing certain configuration files
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.1-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Jamie Gritton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-03 06:27 UTC by Markus Stoff
Modified: 2020-09-04 00:24 UTC (History)
5 users (show)

See Also:


Attachments
proposed patch for jail (543 bytes, patch)
2020-08-15 17:49 UTC, Akos Somfai
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Stoff 2020-08-03 06:27:34 UTC
Some variable names may cause the configuration parser to crash. So far I could only reproduce this issue with "$interface".

Example:

Setting "$interface" in two jail configurations ("crash" and "nocrash"). Using any jail configuration but the last one ("nocrash") will crash the configuration parser. You could have any number of crashing jails ("crash1", "crash2", ...), but only one jail that does not crash (the bottom most one).

jail.conf:

    persist;
    exec.prestart = "echo '\$interface = ${interface}'";
    
    crash {
            $interface = "vr0";
    }
 
    nocrash {
            $interface = "vr1";
    }

How to reproduce:

    # jail -f jail.conf -c crash
    Segmentation fault (core dumped)

    # jail -f crash.conf -c nocrash
    $interface = vr1
    nocrash: created
    # jail -f crash.conf -r nocrash
    nocrash: removed
Comment 1 Akos Somfai 2020-08-15 17:49:47 UTC
Created attachment 217233 [details]
proposed patch for jail

The issue is seen every time when the defined variable ("$interface" in the bug report) is the same as one of the built-in jail.conf parameters excluding the leading "$". The crash is a use-after-free as variable data is free-ed at a point but referenced later from intparams.

Having a variable with the same name as a built-in one is problematic anyways -- the fix eliminates the crash and treats such entries as pure variables as expected by the leading "$". This is also according to the jail.conf description that says that "variables are  only used for substitution, while parameters are used both	for substitution and for passing to the kernel."
Comment 2 commit-hook freebsd_committer 2020-08-26 00:43:38 UTC
A commit references this bug:

Author: jamie
Date: Wed Aug 26 00:43:00 UTC 2020
New revision: 364791
URL: https://svnweb.freebsd.org/changeset/base/364791

Log:
  Handle jail.conf variables that have the same names as parameters.

  PR:		248444
  Submitted by:	Akos Somfai
  Reported by:	Markus Stoff

Changes:
  head/usr.sbin/jail/config.c
Comment 3 commit-hook freebsd_committer 2020-08-26 18:36:35 UTC
A commit references this bug:

Author: jamie
Date: Wed Aug 26 18:35:33 UTC 2020
New revision: 364828
URL: https://svnweb.freebsd.org/changeset/base/364828

Log:
  Back out r364791 to unbreak jails.  Lesson learned: "compile and test" means
  running the test on the same executable that you just compiled.

  PR:		248444
  Pointy hat to:	jamie

Changes:
  head/usr.sbin/jail/config.c
Comment 4 Jamie Gritton freebsd_committer 2020-08-26 21:40:31 UTC
OK, so that obviously didn't work out.  I'm going to go with just catching these things early: as mentioned, variables with the same names as parameters a problematic.  While a core dump isn't a good way of handling them, an error message can be.
Comment 5 commit-hook freebsd_committer 2020-08-27 00:17:23 UTC
A commit references this bug:

Author: jamie
Date: Thu Aug 27 00:17:17 UTC 2020
New revision: 364850
URL: https://svnweb.freebsd.org/changeset/base/364850

Log:
  Don't allow jail.conf variables to have the same names as jail parameters.
  It was already not allowed in many cases, but crashed instead of giving an
  error.

  PR:		248444

Changes:
  head/usr.sbin/jail/config.c
Comment 6 commit-hook freebsd_committer 2020-09-04 00:23:27 UTC
A commit references this bug:

Author: jamie
Date: Fri Sep  4 00:22:25 UTC 2020
New revision: 365320
URL: https://svnweb.freebsd.org/changeset/base/365320

Log:
  MFC r364850:

      Don't allow jail.conf variables to have the same names as jail parameters.
      It was already not allowed in many cases, but crashed instead of giving an
      error.

  PR:		248444

Changes:
_U  stable/12/
  stable/12/usr.sbin/jail/config.c