[Debconf-devel] Bug#1136114: debconf: validate format and driver names before eval STRING

[Sebastian EM]

Hi Colin,

Thank you again for the detailed guidance.

I took up your suggestion and prepared a revised patch that avoids string
eval for the dynamic debconf plugin-loading paths instead of only
validating immediately before the eval.

The patch introduces a small reusable Debconf::Plugin helper. It validates
the original plugin name before converting it into a module path, then
loads the resulting .pm file with require. I intentionally validate the
original name rather than only the constructed path, since validating the
path alone would still allow names containing / to become valid path
segments after catfile.

The patch currently covers the original Format/DbDriver paths as well as
the related frontend-loading paths you mentioned:

   - Debconf::Db
   - Debconf::DbDriver::{File,Directory,Pipe}
   - Debconf::AutoSelect::make_frontend
   - Debconf::FrontEnd::_loadelementclass

I also adjusted the helper contract so that Debconf::Plugin::load
consistently throws on both validation and require failures. This preserves
the normal caller idiom:

eval { Debconf::Plugin::load(...) };

and keeps $@ visible to existing warning/error paths.

Tests added:

   - valid plugin/module names are accepted;
   - invalid names with path traversal, slashes, dots, spaces, and
   metacharacters are rejected;
   - malformed Format/DbDriver names do not execute constructed Perl code;
   - the existing $@ behavior through caller-side eval is preserved.

Validation performed:

   - git apply --check against pristine upstream: OK
   - perl -c on modified files: OK
   - prove -lr t: 98 tests passing
   - trailing whitespace check: clean

Patch attached:

0001-debconf-avoid-string-eval-when-loading-plugin-module.patch

Please let me know if you would prefer a different module name/location for
the helper or a narrower first patch covering only the Format/DbDriver
paths.

Best regards,
Jeremy

El dom, 10 may 2026 a las 14:37, Colin Watson (<cjwatson at debian.org>)
escribió:

> Control: tag -1 - security
>
> [Please drop team at security from CCs in replies, for the reasons given
> below.]
>
> On Sat, May 09, 2026 at 12:16:30PM -0500, Sebastian EM wrote:
> >Package: debconf
> >Version: 1.5.92
> >Severity: important
> >Tags: security patch
> >X-Debbugs-Cc: team at security.debian.org,
> >debconf-devel at lists.alioth.debian.org
> >
> >Dear debconf maintainers,
> >
> >I would like to report an input-validation issue in debconf 1.5.92,
> >confirmed at runtime in a fresh debian:sid container.
> >
> >Several debconf database driver initialization paths interpolate
> >attacker-influenced format or driver names into Perl eval STRING:
>
> In general, debconf doesn't attempt to implement any kind of security
> boundary, and an unprivileged user being able to control DEBCONF_*
> environment variables would already be a significant problem on its own:
> they would be able to substitute their own answers to debconf questions
> asked by packages, which could have arbitrarily complex consequences.
> Any arrangements that allow users to run debconf in privileged contexts
> _must_ forbid them from setting those environment variables.
>
> Therefore, I'm not inclined to treat poor validation of those
> environment variables, which already must not be allowed to be set at
> all by unprivileged users, as a security problem.  I'm happy to treat it
> as a quality-of-implementation problem, though.
>
> >The patch touches:
> >
> >  Debconf/DbDriver/File.pm
> >  Debconf/DbDriver/Directory.pm
> >  Debconf/DbDriver/Pipe.pm
> >  Debconf/Db.pm
>
> I'd like to see if we can avoid the string eval entirely.  All other
> things being equal, it's usually better to avoid the need for validation
> in the first place; this is analogous to the well-known reasons that
> it's better to avoid system()-style interfaces (where one has to
> sanitize shell metacharacters first) in favour of things with an
> execve()-style interface.
>
> So, how about using this sort of construct (which I've tested lightly at
> an interactive prompt, but not in any detail) instead of string eval?
>
>    use File::Spec;
>
>    my @parts = split /::/, $this->{format};
>    my $module = File::Spec->catfile('Debconf', 'Format', $this->{format});
>    eval { require "$module.pm"; };
>
> Of course, that has a path traversal vulnerability: you could supply a
> format beginning with "../../" and then load whatever module you like.
> (Or add further "../" and potentially get anything on the system,
> although this would be hard to do much with since you'd need something
> with the ".pm" suffix.)  So that becomes something like:
>
>    use File::Spec;
>
>    my @parts = split /::/, $this->{format};
>    my $module = File::Spec->catfile('Debconf', 'Format', $this->{format});
>    if ($module !~ m{\A[A-Za-z0-9/_]+\Z}) {
>            $this->error("Invalid plugin name: $this->{format}");
>            return;
>    }
>    eval { require "$module.pm"; };
>
> Now, that's taken us right back to doing up-front validation, which I
> just said I wanted to avoid!  Still, I think it's worth avoiding string
> eval on general principles anyway, so maybe this is worth the effort.
>
> This is enough code that it would definitely need to go into a utility
> module somewhere.  There's no very obvious existing module that would
> suit, but if the code were generalized a little bit then it could go
> into a new Debconf::Plugin module.  As well as the code your patch
> already touches, this could potentially be used in
> Debconf::AutoSelect::make_frontend and
> Debconf::FrontEnd::_loadelementclass.
>
> What do you think?  If this is too much refactoring then I'm happy to do
> it, but I thought I'd give you the opportunity.
>
> Thanks,
>
> --
> Colin Watson (he/him)                              [cjwatson at debian.org]
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/debconf-devel/attachments/20260510/ae0bde27/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-debconf-avoid-string-eval-when-loading-plugin-module.patch
Type: application/octet-stream
Size: 21755 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/debconf-devel/attachments/20260510/ae0bde27/attachment-0001.obj>