LoadFile returns tainted data [rt.cpan.org #35522]

[toddr]

Migrated from rt.cpan.org#35522 (status was 'open')

Requestors:

• [email protected]

From [email protected] on 2008-04-29 19:54:39:

In chasing a CPAN::PERL5INC bug, it became apparent that YAML::Syck was
tainting data returned from LoadFile and YAML/YAML::Tiny weren't.  Not
sure what the right answer is, but the inconsistency should be
addressed.  Excerpt from my email about it to Schwern, Andreas and
interested parties follows:

> Sorry -- I wasn't clear.  CPAN::PERL5INC just gets a list of
> directories from a YAML data file and unshifts them to @INC.  It can
> use any of the YAML modules that provide LoadFile().
> 
> The issue appears to be that YAML::Syck returns a tainted data
> structure.  It doesn't happen with YAML or YAML::Tiny.  I would
> presume that YAML and YAML::Tiny use regexes to parse the YAML file
> and that leads to an untainted structure.

From [email protected] on 2010-05-20 11:28:25:

On Tue Apr 29 15:54:39 2008, DAGOLDEN wrote:
> In chasing a CPAN::PERL5INC bug, it became apparent that YAML::Syck 
was
> tainting data returned from LoadFile and YAML/YAML::Tiny weren't.  Not
> sure what the right answer is, but the inconsistency should be
> addressed.  Excerpt from my email about it to Schwern, Andreas and
> interested parties follows:
> 
> > Sorry -- I wasn't clear.  CPAN::PERL5INC just gets a list of
> > directories from a YAML data file and unshifts them to @INC.  It can
> > use any of the YAML modules that provide LoadFile().
> > 
> > The issue appears to be that YAML::Syck returns a tainted data
> > structure.  It doesn't happen with YAML or YAML::Tiny.  I would
> > presume that YAML and YAML::Tiny use regexes to parse the YAML file
> > and that leads to an untainted structure.

I guess I could wrap Load* in Taint::Util::untaint(). Anyway, tests 
would be welcome. How do modules that normally parse things handle stuff 
under -T? Isn't it pretty incidental what is tainted and what not? Since 
filehandles are tainted by default and thus it's pretty much an 
implementation detail whether something gets untainted (i.e. whether it 
used regexes along the way).

(This is a form-reply that isn't specific to your particular report)

YAML::Syck has just acquired one new maintainer (me), it still doesn't
have anyone that *cares* about it. But I'm willing to help solve your
report & release a new version with the fix if it's easy for me.

It now has a Git repository at:

    http://github.com/avar/YAML-Syck

If your report is a patch that fixes a problem, great. Please remake
the patch against Git by forking that repo and sending me a pull
request on GitHub (or an update to this bug if you prefer
git-format-patch(1) or some other repo provider..). Make sure to
include a test for what you fixed.

If your report is some code that fails (and you have a testcase for
it) a patch against the test suite to demonstrate that failure would
be very useful. It's OK if the test crashes and burns, see
Test::More's docs for how to make TODO tests that fail now, but
shouldn't. Even if it segfaults perl C<system $^X => qw/ -Mblib
-MYAML::Syck .../> or something like that and checking the return
value will do.

From [email protected] on 2010-07-19 20:57:20:

As I read this, it seems the implication is that YAML::Syck might be doing it right by tainting and 
the ones that don't might be doing it wrong. Is this something you want to see fixed? Or is 
CPAN now untainting the results on it's own?

From [email protected] on 2010-07-19 21:06:44:

On Mon, Jul 19, 2010 at 1:57 PM, Todd Rinaldo via RT
<[email protected]> wrote:
> <URL: https://rt.cpan.org/Ticket/Display.html?id=35522 >
>
> As I read this, it seems the implication is that YAML::Syck might be doing it right by tainting and
> the ones that don't might be doing it wrong. Is this something you want to see fixed? Or is
> CPAN now untainting the results on it's own?

I don't think it's very useful to have a tainted data structure in
typical usage, but the bug report was mostly that the various YAML::*
modules all provide a "compatible" API and having YAML::Syck produce
tainted data causes problems because of that inconsistency.

-- David

From [email protected] on 2010-07-19 21:09:30:

On Mon Jul 19 17:06:44 2010, DAGOLDEN wrote:
> On Mon, Jul 19, 2010 at 1:57 PM, Todd Rinaldo via RT
> <[email protected]> wrote:
> > <URL: https://rt.cpan.org/Ticket/Display.html?id=35522 >
> >
> > As I read this, it seems the implication is that YAML::Syck might be
> doing it right by tainting and
> > the ones that don't might be doing it wrong. Is this something you
> want to see fixed? Or is
> > CPAN now untainting the results on it's own?
> 
> I don't think it's very useful to have a tainted data structure in
> typical usage, but the bug report was mostly that the various YAML::*
> modules all provide a "compatible" API and having YAML::Syck produce
> tainted data causes problems because of that inconsistency.

I understand. Compatibility is nice in general, but it does sound like 
whatever code is assuming that the data from YAML::* isn't tainted is 
broken in the first place. Whether something gets untainted for you is 
really incidental, but maybe the other YAML::* maintainers made a conscious decision on this.

What breaks because YAML::Syck doesn't auto-untaint under taint mode?