Have stricter rules on chroot names [CVE-2022-2787]

Commit 8c1c9370 ("sbuild::chroot_config: Replace is_valid_filename with
is_valid_sessionname") introduced a very broad rule on the name of a
chroot or session name. As it turned out, some of the allowed characters
would break schroot's internal state, or would make the setup croak in
various backends.

This change limits the set of allowed characters to what seems to be
safe: Letters and digits, and in all but the first place, the characters
dot ('.'), dash ('-'), and underscore ('_').

Special thanks to Julian Gilbey <jdg@debian.org> for reporting the issue
and providing additional insights about the severity.

NEWS

@@ -9,7 +9,38 @@ configuration.
 
 * Major changes in 1.6.13:
 
-  1)
+  1) The rule on allowed characters in a chroot or session is a lot
+     stricter now.  This is required to address CVE-2022-2787.
+
+     Before upgrading, you should check the output of
+
+          schroot --list --all | LC_ALL=C grep -vE '^[a-z]+:[a-zA-Z0-9][a-zA-Z0-9_.-]*$'
+
+     Any chroot or session listed here will become invisible, so you're
+     strongly advised to stop or rename them.
+
+
+     Dealing with an affected chroot:
+
+     To rename, edit the appropriate files, this is either
+     /etc/schroot/schroot.conf or in /etc/schroot/chroot.d/*. Change
+     the name in square brackets and, if needed, the "aliases=..."
+     line.
+
+
+     Dealing with an affected session:
+
+     Consider ending the session (--end-session) before upgrading.
+     This is the only safe way, especially if the problem is the name
+     of the underlying chroot.
+
+     The following steps should do the trick but still might break the
+     system:
+     (1) Edit /var/lib/schroot/session/<sessionname>, changing the first
+         line with the name in square brackets, the "name=..." line and,
+         if needed, the "aliases=..." line.
+     (2) Also rename /var/lib/schroot/session/<oldsessionname> to
+         /var/lib/schroot/session/<newsessionname>
 
 * Major changes in 1.6.12:
 

man/schroot.conf.5.man

@@ -547,37 +547,19 @@ description[fr]=\f[CI]French translation\fP
 .PP
 This will localise the \f[CI]description\fP key for all French locales.
 .SH CHROOT NAMES
-A number of characters or words are not permitted in a chroot name, session
-name or configuration filename.  The name may not contain a leading period
-(\[oq].\[cq]).  The characters \[oq]:\[cq] (colon), \[oq],\[cq] (comma) and
-\[oq]/\[cq] (forward slash) are not permitted anywhere in the name.  The name
-may also not contain a trailing tilde (\[oq]~\[cq]).  The rationale for these
-restrictions is given below.
+A number of characters or words are not permitted in a chroot name,
+session name or configuration filename.  The name must begin with a
+lowercase or an uppercase letter, or a digit.  The remaining characters
+may additionally be dash (\[oq]-\[cq]), period (\[oq].\[cq]), or
+underscore (\[oq]_\[cq]).
+.PP
+The rationale for these restrictions is as follows:
 .TP
-.RB \[oq] . \[cq]
-A leading period could be used to create a name with a relative path in it, in
-combination with \[oq]/\[cq], and this could allow overwriting of files on the
-host filesystem.  Not allowing this character also means hidden files cannot be
-created.  It also means some editor backups are automatically ignored.  Periods
-are allowed anywhere else in the name.
-.TP
-.RB \[oq] : \[cq]
-A colon is used as a namespace delimiter, and so is not permitted as part of a
-chroot or session name.  LVM snapshot names may also not contain this character
-due to a naming restriction by
-.BR lvcreate (8).
-.TP
-.RB \[oq] / \[cq]
-Names containing this character are not valid filenames.  A forward slash would
-potentially allow creation of files in subdirectories.
-.TP
-.RB \[oq] , \[cq]
-Commas are used to separate items in lists.  Aliases are separated by commas
-and hence can't contain commas in their name.
-.TP
-.RB \[oq] ~ \[cq]
-Filenames containing trailing tildes are used for editor backup files, which
-are ignored.  Tildes are allowed anywhere else in the name.
+.RB Generic
+Unfortunately, not all the places that deal with chroot names can
+handle non-printable and other characters properly, and it's hard to
+update all of them.  This is mostly about the various shell scripts
+where it's also unwise to assume authors always create safe code.
 .TP
 .RB \[oq] dpkg-old \[cq]
 .TQ

sbuild/sbuild-util.cc

@@ -174,12 +174,11 @@ sbuild::is_valid_sessionname (std::string const& name)
 {
   bool match = false;
 
-  static regex file_namespace("^[^:/,.][^:/,]*$");
-  static regex editor_backup("~$");
+  // keep in sync with schroot.conf(5)
+  static regex file_namespace("^[a-zA-Z0-9][a-zA-Z0-9_.-]*$");
   static regex debian_dpkg_conffile_cruft("dpkg-(old|dist|new|tmp)$");
 
   if (regex_search(name, file_namespace) &&
-      !regex_search(name, editor_backup) &&
       !regex_search(name, debian_dpkg_conffile_cruft)) {
     match = true;
   }