CODEWITHSUNDEEP
regexperlsecurity
Joe
Joe

Reputation: 4514

I believe this Perl script is secure. Can it be improved?

I have the following Perl script. One can (not right now because I just took it down) ping a URL like

http://www.joereddington.com/testsound/getsound.pl?text=hello%20mum

and then find that the file

http://www.joereddington.com/testsound/hope.wav

is a recording of a computer voice saying "hello mum". 

#!/usr/bin/perl
use strict;
use warnings;

use CGI qw(:standard -debug);

my $text = param('text');
$text =~ s/[^0-9a-zA-Z\s]//g;

print "Content-type: text/html\n\n";

system("/home8/projedf4/tts/espeak-1.48.04-source/src/speak \"$text\" -w hope.wav");

I'm a little nervous about having users possibly take advantage of injection attacks and the like. I believe I've done enough with the line

$text =~ s/[^0-9a-zA-Z\s]//g;

because I'm just extracting literally everything that could do damage from the string.

But is this enough? I could even go as far as

$text =~ s/[^0-9a-zA-Z\s\.,]//g;

Upvotes: 1

Views: 105

Answers (1)

ikegami
ikegami

Reputation: 386541

Yeah, your code is fine (ignoring bugs in perl, speak, DOS attacks, etc.), assuming the only arguments that are special to speak start with -.

But it can be improved.

  1. There's no need to remove so many characters.

    sub shell_quote {
   return map {
      die if /\x00/;
      my $lit = $_;
      $lit =~ s/'/'\\''/g;
      "'$lit'"              
   } @_;
}

$text =~ s/^-+//;
system(shell_quote('/.../speak', $text, '-w', 'hope.wav'));

    or

    use String::ShellQuote qw( shell_quote );

$text =~ s/^-+//;
system(shell_quote('/.../speak', $text, '-w', 'hope.wav'));

  2. There's no need to launch a shell too.

    die if $text =~ /\x00/;
$text =~ s/^-+//;
system('/.../speak', $text, '-w', 'hope.wav');

  3. If your speak supports --, you could even use

    die if $text =~ /\x00/;
system('/.../speak', '-w', 'hope.wav', '--', $text);

Upvotes: 6

Related Questions