$month = $month + 1;    #localtime returns mth as 0 to 11
    $outfile = sprintf "%s/%4d-%2.2d-%2.2d-", $base_dir, 
                $year + 1900, $month, $day;
[download]

That's what the -T in your shebang (firstline) is about.

The line you get the error involves $outfile.

$outfile equates to (for example) - 2003-12-18-0001133 , for order number 1133, the preceeding stuff is a date stamp (we have the timestamp from the website, but the filename is unique this way)

Where are $basedir, $month, $year and $day from?

$basedir is set once at the top of the Perl script, just after the shebang code I posted. $month, $year and $day are _only_ used in the code/sub I supplied (the 4 lines) They are 'retrieved' from the system/website.

Have you untainted your foreign source (user input, stream input, file input) variables? If you have NO clue what I mean, then i suggest doing a "man perlsec" and reading up on the taint section. They talk about untainting and the likes.

Yes, I have no idea what you mean. There is a lot of foreign source coming from the browser form. I'll have to read up on 'taint'; the shell access on the website takes 3 cups of coffee to 'work', but I also have Active Sate perl , and I've found 'perlsec', so I'll try and digest that over the next few days

Possibly one thing to consider is this perl script (now 1064 lines) was inherited from a Unix box. Maybe Perl worked _slightly_ differently there ??

Thanks,

Peter

So there is your problem Read up on the man page, it's far more comprehensive compared to what I'll cut-paste for you....

From the man page

The only way to bypass the tainting mechanism is by referencing subpatterns from a regular expres- sion match. Perl presumes that if you reference a substring using $1, $2, etc., that you knew what you were doing when you wrote the pattern. That means using a bit of thought--don't just blindly untaint anything, or you defeat the entire mechanism. It's better to verify that the variable has only good characters (for certain values of "good") rather than checking whether it has any bad characters. That's because it's far too easy to miss bad characters that you never thought of.

If $input is 12345, and 12345 comes from $cgi->param(....), then a simple regexp pattern of...

if($input=~/(\d+)/) { $input = $1; }
else{ $input = undef }
[download]

Your regexp should test and assign out the various ()'s. You can do 3 ()'s at a time, or just one... but only assign $1... $9 ONLY if your regexp matches. doing $input=~/(...)/ and then $input = $1 without an if statement might screw things up BIG time. $1 gets assigned on a successful match. It doesn't get undef'd if a match is unsuccessful.

You don't have to copy $1 back to $input either. I only did it to create less variables. You can do $verifiedInput = $1 too. But remember to program around the cases when the regexp matches or doesn't.


Play that funky music white boy..

Possibly one thing to consider is this perl script (now 1064 lines) was inherited from a Unix box. Maybe Perl worked _slightly_ differently there ??

I imagine that perl worked significantly different there as there are many perl built-ins that don't exist on Win32 :-) But I don't think that's relevant to your problem. It just looks like you have possibly multiple sources of tainted data that you need to untaint before using. The cannonical way to untaint your data is by using a regular expression. For example, your $seq variable is read from a file and is supposed to be a number but perl has no way of knowing this; it's just text read from a file. So, to untaint it:

($seq) = $seq =~ /(\d+)/;     # Just grab the first run of digits
[download]

duff

It sounds like taint mode doesn't like $base_dir. That could mean the variable is tainted, or perhaps that it is a relative path and $ENV{PWD} is.

Btw, you can save trouble with your time formatting by calling POSIX::strftime($format, localtime).

It sounds like taint mode doesn't like $base_dir. That could mean the variable is tainted, or perhaps that it is a relative path and $ENV{PWD} is.

Here is where $base_dir is defined

#!/usr/bin/perl -wT

use CGI qw/:standard/;
use DBI;
use Net::SMTP;
#use CGI::Carp qw(fatalsToBrowser);    #comment this out when in produ
+ction

BEGIN {
     use CGI::Carp qw(carpout);
     
     open(LOG, ">>/home/username/cgi-bin/process-log") or
       die("Unable to open process-log: $!\n");
     carpout(LOG);
   }

# resource limits
$CGI::DISABLE_UPLOADS = 1;  # no uploads
$CGI::POST_MAX = 1024 * 10;  # max 10K posts

# settings
$base_dir = '/home/username/.orders';
[download]

Btw, you can save trouble with your time formatting by calling POSIX::strftime($format, localtime).

Okay, thanks, no doubt I can just place it where the current timestamps are evaluated.

Peter

If all you've said is accurate, it looks to me like $outfile is the problem since it's concatenated with $seq and $seq is tainted. (It's read from a file but not untainted before being concatenated). As sporty said, read perlsec.

If all you've said is accurate, it looks to me like $outfile is the problem since it's concatenated with $seq and $seq is tainted. (It's read from a file but not untainted before being concatenated).

Yes, $seq is evaluated from the file called .SEQ , and then $outfile is evaluated by appending the value of $seq to the previously evaluated $outfile (a datestamp format).

But I have never seen this message before ?

As sporty said, read perlsec

Okay, will do !! :)

Peter

But I have never seen this message before

I can not speak to that. There are many variables that could cause this particular taint message to never have surfaced before, not the least of which could be bugs in perl's taint checking. Or it could be that you've always started with $seq = 0; from the other branch of your if statement for some reason. Or that at one point there was a line to untaint $seq but it was removed for some reason. Or something else that no one has yet thought of.

duff

Well, I was able to reproduce the error message. Even removing the 'line feeds' from the ".SEQ" file had no effect, the same error message appeared in the log file. I have no idea why this problem has suddenly appeared, as the Perl script hasn't been changed in months, and we have had orders go through since then ? Hmm, I'll check out the web hosts, to see if they, ........yikes ....

"Perl has been upgraded to version 5.8.1 from version 5.6.1"

This was done 2 days prior to the error msg appearing, so I have to assume that Perl 5.8.1 is, like a lot of new versions, "stricter" with how it handles data, especially tainted, if that correct ??

I didn't really know where to place the line to do the untaint, but here is the new sub

sub create_order_file {
        
my (undef, undef, undef, $day, $month, $year, @rest) = localtime(time)
+;
    $month = $month + 1;    #localtime returns mth as 0 to 11
    $outfile = sprintf "%s/%4d-%2.2d-%2.2d-", $base_dir, 
                $year + 1900, $month, $day;
            
    umask(); -d $base_dir || mkdir $base_dir, 0700;
    
my $got_lock;
use Fcntl;   # to get constants for O_CREAT | O_EXCL | O_RDWR
for ( 0 .. 5 ) {
    if ( sysopen(my $fh, "$base_dir/.lock", O_CREAT | O_EXCL | O_RDWR,
+ 0600) ) {
        $got_lock = 1;
        close $fh;
      last;
    }
    sleep 2;
}
diehtml("Lock error $!\n") unless $got_lock;    
        
    # create unique suffix
    if (-f "$base_dir/$seq_file") {
        open(SEQ, "+<$base_dir/$seq_file")
            or diehtml("Error opening seq file: $!\n");
        $seq = <SEQ>;
        seek SEQ, 0, 0;
    } else {
        open(SEQ, ">$base_dir/$seq_file")
            or diehtml("Error creating seq file: $!\n");
        $seq = 0;
    }
    
    ($seq) = $seq =~ /(\d+)/;     # Just grab the first run of digits
    $outfile .= sprintf "%7.7d", $seq;
    $ordernumber = $seq;    #store this order no. before 'next' is cal
+c.
    print SEQ ++$seq;
    close SEQ or warn "Something wrong closing seq: $!\n";
    unlink "$base_dir/.lock" or diehtml("Unlock error: $!\n");

use Fcntl;   # to get constants for O_CREAT | O_EXCL | O_RDWR
    
        sysopen(ORDERFILE, $outfile, O_WRONLY | O_EXCL | O_CREAT)
            or diehtml("Can't open order records: $!\n");

    print ORDERFILE @_;
    close ORDERFILE or warn "Something fishy with closing the order: $
+!\n";
    
}
[download]

The line added is ($seq) = $seq =~ /(\d+)/; # Just grab the first run of digits, hopefully that was the right place to put it. Anyway, the script worked just fine this time. :)

What I didn't understand though, is when I reproduced the error, it crashed at the same line, where sysopen is, but the browser just had a blank page, nothing displayed at all ? I would have thought I'd see:

"Can't open order records:" displayed, but nothing was ??

The sub "diehtml" is

sub diehtml {
    print start_html('Error processing order'), @_, end_html(), "\n";
    exit 1;
}
[download]

I have no idea why there was no message, maybe something to do with using CGI::Carp ??

Thanks to everyone for your help. Possibly I'd better warn other people on the web hosts about the potential problem

Peter

I noticed from search.cpan.org: perl573delta - what's new for perl v5.7.3 , the following

11410 - fix a bug in the security taint checking of open()

13684 - introduce the -t option for gentler taint checking

As the version jump was from 5.6.1 to 5.8.1 , and the script crashed at a "sysopen", I assume this is related ?

Peter

This article, at http://twiki.org/cgi-bin/view/Support/ApacheUpgradeTaintError

is also interesting, same version of Perl, same error message, and the following answer

"It looks like =Net::SMTP and/or IO::Socket got more strict in regards to taint checking."

The script that crashed uses Net::SMTP, but it hadn't got into _that_ subroutine when it crashed.

Peter