Creating a scalar value from other scalars

R

Robert TV

Hi, I thought I read somewhere once that creating a scalar value from other
scalars is bad. My small script below creates a data based on the users IP
address and the current Epoch. I'm asking for advice on whether this is bad
scripting.

__ACTUAL CODE__

#!/usr/bin/perl

use CGI qw/:standard/;
use CGI::Carp qw(fatalsToBrowser);

$timestamp = time();
$ipaddress = $ENV{'REMOTE_ADDR'};
$ipaddress =~ s/\.//g; #remove periods from IP address
$filename = "$ipaddress\_$timestamp\.dat";

open (FILE,">>$filename"); #create new filename based on above variables
close(FILE);

print header;
print "File name: $filename has been created on server";
exit;


__END CODE__

The above creates a new file on the server with a name similair to
"20553102224_1087758244.dat". While the code functions without error, is the
scripting sound? I would appreciate alternative methods if this script is
inefficient or erroneous. Thank you.

Robert
 
B

Ben Morrow

Quoth "Robert TV said:
Hi, I thought I read somewhere once that creating a scalar value from other
scalars is bad.

I have no idea where you heard that, but it is total nonsense. Most of
programming consists of creating variables from the values of other
variables...
My small script below creates a data based on the users IP
address and the current Epoch. I'm asking for advice on whether this is bad
scripting.

__ACTUAL CODE__

#!/usr/bin/perl

You should probably use taint mode for CGI scripts. See perldoc perlsec.

You also need

use warnings;
use strict;

here.
use CGI qw/:standard/;
use CGI::Carp qw(fatalsToBrowser);

$timestamp = time();

my $timestamp = time;

as you're now using strict. Ditto below.
$ipaddress = $ENV{'REMOTE_ADDR'};

You will need to untaint this before you can create a file based on it.
A good solution would be to use Regexp::Common.
$ipaddress =~ s/\.//g; #remove periods from IP address
$filename = "$ipaddress\_$timestamp\.dat";

There is no need to escape a . in strings; I would use ${} rather than
escaping the backslash:

my $filename = "${ipaddress}_$timestamp.dat";
open (FILE,">>$filename"); #create new filename based on above variables

ALWAYS check that open succeeded.
Use three-arg open.
Use lexical filehandles.

{
open my $FILE, '>>', $filename
or die "can't append to $filename: $!";
}
close(FILE);

As $FILE is a lexical, it will be automatically closed at the end of the
scope.
print header;
print "File name: $filename has been created on server";
exit;

__END CODE__

If you just write __END__ then perl will understand you as well...

Ben
 
J

John Bokma

Robert said:
Hi, I thought I read somewhere once that creating a scalar value from other
scalars is bad. My small script below creates a data based on the users IP
address and the current Epoch. I'm asking for advice on whether this is bad
scripting.

Your script is bad.
__ACTUAL CODE__

#!/usr/bin/perl

I recommend adding -T to the she-bang. Look up taint mode

Also add:

use strict;
use warnings;
use CGI qw/:standard/;
use CGI::Carp qw(fatalsToBrowser);

$timestamp = time();

my $timestamp;

etc.
$ipaddress = $ENV{'REMOTE_ADDR'};
$ipaddress =~ s/\.//g; #remove periods from IP address
$filename = "$ipaddress\_$timestamp\.dat";

open (FILE,">>$filename"); #create new filename based on above variables

open can fail, check this.

MORE IMPORTANT: you build a filename from external DATA. CHECK THAT IT'S
VALID, and more important don't fix it, reject it if it's not valid. So
clearly define what you consider valid and secure.
close(FILE);

can fail, check, and hence make use of the CGI mods.
 
P

Paul Lalli

Hi, I thought I read somewhere once that creating a scalar value from other
scalars is bad. My small script below creates a data based on the users IP
address and the current Epoch. I'm asking for advice on whether this is bad
scripting.

I think it's possible you misunderstood what you read. My impression is
that you saw a conversation involving creating variable _names_ from other
variables, not the actual variables themselves. This is bad. An example
of this is:

$name = 'John';
${$name} = 23;

Using this code, a new variable $John springs into existence, with the
value of 23. This is called Symbollic References. They are very much
considered bad. For a thorough discussion of why, run the following
command:

perldoc -q 'variable name'


What you were doing, however, is creating variable *values* based on
existing variables. This is not only not wrong, it's one of the most
basic foundations of programming.

I hope this helps clarify. Or maybe I guessed completely wrong. It's
been known to happen. :)

Paul Lalli
 
A

Alan Curry

$ipaddress =~ s/\.//g; #remove periods from IP address
$filename = "$ipaddress\_$timestamp\.dat";

Why remove the dots? If you're interested in IP addresses, you probably don't
want to treat 12.122.11.50 (gbr1-p40.cgcil.ip.att.net) and 12.12.211.50
(50.juneau-05rs16rt.ak.dial-access.att.net) as identical.

Dots in filenames are not normally a problem. If they're a problem for you,
replace them with underscores, convert the whole IP address into a single
number like this:

use Socket;
my $longipaddress = unpack "N", inet_aton($ipaddress);
# Or even this:
my $hexipaddress = sprintf "%08x", unpack "N", inet_aton($ipaddress);
 
B

Ben Morrow

Quoth (e-mail address removed) (Alan Curry):
Why remove the dots? If you're interested in IP addresses, you probably don't
want to treat 12.122.11.50 (gbr1-p40.cgcil.ip.att.net) and 12.12.211.50
(50.juneau-05rs16rt.ak.dial-access.att.net) as identical.

You should also note that this value will not uniquely identify a given
request: it is perfectly possible for many requests to arrive from the
same ip in a second, especially if it is a NAT box with several hosts
behind it.

Ben
 
J

John Bokma

Purl said:
You are the first today to respond who has taken notice
of this inherent race condition, a race which will be
won, many times per second, resulting in corrupted and
inaccurate data.

Since the OP only creates a file, there is no race condition :-D
There are other serious issues, as well.

Like... hackers at your site ;-) ROTFLMAO.
 
J

Joe Smith

Robert said:
I read somewhere once that creating a scalar value from other scalars is bad.

Using tainted data is bad. Anything that comes from %ENV or @ARGV or from
CGI is tainted. Using such data without untainting it can allow an outside
user to walk all over your data. Changing the first line of your script to
"#!/usr/bin/perl -T" will enable taint checking.
$ipaddress = $ENV{'REMOTE_ADDR'};
$ipaddress =~ s/\.//g; #remove periods from IP address
$filename = "$ipaddress\_$timestamp\.dat";

And what happens if $ENV{REMOTE_ADDR} happens to be '..\..\..\win32\system'?
[Actually, REMOTE_ADDR can probably be trusted as its value comes from
the web server. But many other values in %ENV cannot be trusted.]

Instead of stripping dots, you could use the corresponding 32-bit integer.

use Socket;
$ip_numeric = unpack "N", inet_aton $ENV{REMOTE_ADDR};

That will keep '12.122.1.50' (with a value of 209324850) distinct from
'12.12.211.50' (which has a value of 202167090).

-Joe
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,483
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top