Creating a scalar value from other scalars

Discussion in 'Perl Misc' started by Robert TV, Jun 20, 2004.

  1. Robert TV

    Robert TV Guest

    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
     
    Robert TV, Jun 20, 2004
    #1
    1. Advertising

  2. Robert TV

    Ben Morrow Guest

    Quoth "Robert TV" <>:
    > 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

    --
    I must not fear. Fear is the mind-killer. I will face my fear and
    I will let it pass through me. When the fear is gone there will be
    nothing. Only I will remain.
    Frank Herbert, 'Dune'
     
    Ben Morrow, Jun 20, 2004
    #2
    1. Advertising

  3. Robert TV

    John Bokma Guest

    Robert TV wrote:

    > 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.

    --
    John MexIT: http://johnbokma.com/mexit/
    personal page: http://johnbokma.com/
    Experienced Perl programmer available: http://castleamber.com/
    Happy Customers: http://castleamber.com/testimonials.html
     
    John Bokma, Jun 20, 2004
    #3
  4. Robert TV

    Paul Lalli Guest

    On Sun, 20 Jun 2004, Robert TV wrote:

    > 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
     
    Paul Lalli, Jun 20, 2004
    #4
  5. Robert TV

    Alan Curry Guest

    In article <gplBc.832453$Ig.490304@pd7tw2no>,
    Robert TV <> wrote:
    >$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);

    --
    Alan Curry
     
    Alan Curry, Jun 21, 2004
    #5
  6. Robert TV

    Ben Morrow Guest

    Quoth (Alan Curry):
    > In article <gplBc.832453$Ig.490304@pd7tw2no>,
    > Robert TV <> wrote:
    > >$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.


    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

    --
    "If a book is worth reading when you are six, *
    it is worth reading when you are sixty." - C.S.Lewis
     
    Ben Morrow, Jun 21, 2004
    #6
  7. Robert TV

    John Bokma Guest

    Purl Gurl wrote:

    > Ben Morrow wrote:
    >
    >>Alan Curry wrote:
    >>
    >>>Robert TV wrote:

    >
    >>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.

    >
    > 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.

    --
    John MexIT: http://johnbokma.com/mexit/
    personal page: http://johnbokma.com/
    Experienced Perl programmer available: http://castleamber.com/
    Happy Customers: http://castleamber.com/testimonials.html
     
    John Bokma, Jun 21, 2004
    #7
  8. Robert TV

    Joe Smith Guest

    Robert TV wrote:

    > 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
     
    Joe Smith, Jun 21, 2004
    #8
    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. Ray Gardener

    Inheriting scalars

    Ray Gardener, Jun 9, 2004, in forum: C++
    Replies:
    7
    Views:
    428
    Ray Gardener
    Jun 10, 2004
  2. Replies:
    8
    Views:
    326
    Paul Boddie
    Mar 1, 2005
  3. ago
    Replies:
    3
    Views:
    327
  4. Clint Olsen
    Replies:
    6
    Views:
    362
    Jeff 'japhy' Pinyan
    Nov 13, 2003
  5. Mark

    Replace scalar in another scalar

    Mark, Jan 27, 2005, in forum: Perl Misc
    Replies:
    4
    Views:
    164
    Arndt Jonasson
    Jan 27, 2005
Loading...

Share This Page