Re: Is this secure
+ACI-Gunther Birznieks+ACI- wrote: +AD4- There are probably multiple issues with this script. I don't really have +AD4- the time to do a security audit for you but in a 5 minute glance +AD4- +AD4- A) -t is supposed to be -T if you are enabling taint mode Doh+ACE- Missed that one. +AD4- B) It appears as if there is very little checking done on the path that is +AD4- issued. Things like +AD4-escaped periods would allow backtracking +AD4-possible null byte insertion in the regex would obviate the file extension +AD4- C) File open is done without explicitly putting in a +ACIAPAAi- prefix to indicate +AD4- read-only access. So if the path starts or ends with +AHw- then an arbitrary +AD4- command could be executed. I couldn't introduce errors using the null-byte, but I won't stop testing just yet. I have no idea to escape a period on the URL. Could someone give me an example please. +AD4- Some of these might not work in practice, but I don't see an explicit area +AD4- of the code which basically prevents these things from occuring, so I can +AD4- only suspect it is possible with enough diligence. +AD4- +AD4- I would suggest that if your site is using mod+AF8-perl, don't use some +AD4- home-grown template system. There are way too many out there that are +AD4- reallly well written and well-tested and examined for security. State your +AD4- requirements and ask the mod-perl list for some advice. Thanks, but nope, this is a test project. +AD4- There are really powerful ones like TemplateToolkit, Mason, EmbPerl, but +AD4- then there are simpler ones also. And the +ACM-1 thing is that if you see +AD4- someone trying to roll their own template system, STOP THEM+ACEAIQ- :) +AD4- +AD4- It's really annoying to reinvent the wheel that's already been reinvented +AD4- many times. No offense, but I don't want to +ACI-stand on the shoulders of giants.+ACI- Part of the reason I'm doing this is to understand the security issues involved, even in a simple script like this. I know there is always more to learn, now I know /what/ I need to learn. cya, Jon +AD4- Later, +AD4- Gunther +AD4- +AD4- At 01:18 AM 2/14/2002, Rednecktek wrote: +AD4- +AD4-I've been asked if this script is secure. I believe it is. Can anyone find +AD4- +AD4-any problems with it? +AD4- +AD4- +AD4- +AD4AIwAh-/usr/bin/perl -w -t +AD4- +AD4-use strict+ADs- +AD4- +AD4-use Apache+ADs- +AD4- +AD4AJA-ENV+AHs-GATEWAY+AF8-INTERFACE+AH0- +AD0Afg- /+AF4-CGI-Perl/ or die ++ACI-GATEWAY+AF8-INTERFACE not Perl+ACEAIgA7- +AD4- +AD4-my +ACQ-r +AD0- Apache-+AD4-request()+ADs- +AD4- +AD4-my +ACU-args +AD0- +ACQ-r-+AD4-args()+ADs- +AD4- +AD4-my +ACQ-path +AD0- +ACQ-r-+AD4-uri+ADs- +AD4- +AD4- +AD4- +AD4AIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACM- +ACM- +AD4- +AD4AIwAjACM- +AD4- +AD4AJA-path +AD0Afg- s/+AFw-/(.+ACo-?)+ACQ-//+ADs- +ACM- Strip off the +scriptname +AD4- +AD4-my +ACQ-tmplpath +AD0- +ACI-template/+ACIAOw- +ACM- Setup the template path +AD4- +AD4-my +ACQ-cont+AF8-ext +AD0- +ACI-.html+ACIAOw- +ACM- Allow only content +files with this extension +AD4- +AD4-my +ACQ-tmpl+AF8-ext +AD0- +ACI-.tmpl+ACIAOw- +ACM- Allow only template +files with this extension +AD4- +AD4-my +ACQ-template +AD0- +ACQ-tmplpath .+ACI-mcti+ACI-. ++ACQ-tmpl+AF8-ext+ADs- +ACM- Setup the template path +AD4- +AD4-my +ACQ-page +AD0- +ACQ-args+AHs-page+AH0- +AHwAfA- +ACI-index+ACIAOw- ++ACM- Are we requesting a page or root? +AD4- +AD4-my +ACQ-title +AD0- +ACI-Microdyne+ACIAOw- +ACM- Default Title of not +specified in page +AD4- +AD4-my +ACQ-debug +AD0- 1+ADs- +AD4- +AD4AIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACM- +ACM- +AD4- +AD4AIwAjACM- +AD4- +AD4-my (+ACQ-content, +ACQ-pageout, +ACQ-newtitle, +ACQ-newtmpl)+ADs- +AD4- +AD4- +AD4- +AD4-(+ACQ-content, +ACQ-newtitle, +ACQ-newtmpl) +AD0- pullpage( +ACQ-page . ++ACQ-cont+AF8-ext )+ADs- +AD4- +AD4-if (+ACQ-newtitle) +AHsAJA-title +AD0- +ACQ-newtitle+ADsAfQ- +AD4- +AD4-if (+ACQ-newtmpl) +AHsAJA-template +AD0- +ACQ-tmplpath . +ACQ-newtmpl . ++ACQ-tmpl+AF8-ext+ADsAfQ- +AD4- +AD4AJA-pageout +AD0- readfile( +ACQ-template )+ADs- +AD4- +AD4AJA-pageout +AD0Afg- s/+ACUAJQ-TITLE+ACUAJQ-/+ACQ-title/g+ADs- +AD4- +AD4AJA-pageout +AD0Afg- s/+ACUAJQ-CONTENT+ACUAJQ-/+ACQ-content/g+ADs- +AD4- +AD4- +AD4- +AD4-pageout(+ACQ-pageout)+ADs- +AD4- +AD4- +AD4- ++AD4AIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAj- +AD4- +AD4AIw- Spit out the content +AD4- ++AD4AIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAjACMAIwAj- +AD4- +AD4-sub pageout +AHs- +AD4- +AD4- my +ACQ-pageout +AD0- shift+ADs- +AD4- +AD4-
Re: Is this secure
There are probably multiple issues with this script. I don't really have the time to do a security audit for you but in a 5 minute glance A) -t is supposed to be -T if you are enabling taint mode B) It appears as if there is very little checking done on the path that is issued. Things like escaped periods would allow backtracking possible null byte insertion in the regex would obviate the file extension C) File open is done without explicitly putting in a "<" prefix to indicate read-only access. So if the path starts or ends with | then an arbitrary command could be executed. Some of these might not work in practice, but I don't see an explicit area of the code which basically prevents these things from occuring, so I can only suspect it is possible with enough diligence. I would suggest that if your site is using mod_perl, don't use some home-grown template system. There are way too many out there that are reallly well written and well-tested and examined for security. State your requirements and ask the mod-perl list for some advice. There are really powerful ones like TemplateToolkit, Mason, EmbPerl, but then there are simpler ones also. And the #1 thing is that if you see someone trying to roll their own template system, STOP THEM!! :) It's really annoying to reinvent the wheel that's already been reinvented many times. Later, Gunther At 01:18 AM 2/14/2002, Rednecktek wrote: >I've been asked if this script is secure. I believe it is. Can anyone find >any problems with it? > >#!/usr/bin/perl -w -t >use strict; >use Apache; >$ENV{GATEWAY_INTERFACE} =~ /^CGI-Perl/ or die "GATEWAY_INTERFACE not Perl!"; >my $r = Apache->request(); >my %args = $r->args(); >my $path = $r->uri; > > >### >$path =~ s/\/(.*?)$//; # Strip off the scriptname >my $tmplpath = "template/"; # Setup the template path >my $cont_ext = ".html"; # Allow only content files with this extension >my $tmpl_ext = ".tmpl"; # Allow only template files with this extension >my $template = $tmplpath ."mcti". $tmpl_ext; # Setup the template path >my $page = $args{page} || "index"; # Are we requesting a page or root? >my $title = "Microdyne"; # Default Title of not specified in page >my $debug = 1; > >### >my ($content, $pageout, $newtitle, $newtmpl); > >($content, $newtitle, $newtmpl) = pullpage( $page . $cont_ext ); >if ($newtitle) {$title = $newtitle;} >if ($newtmpl) {$template = $tmplpath . $newtmpl . $tmpl_ext;} >$pageout = readfile( $template ); >$pageout =~ s/%%TITLE%%/$title/g; >$pageout =~ s/%%CONTENT%%/$content/g; > >pageout($pageout); > > ># Spit out the content > >sub pageout { > my $pageout = shift; > $r->content_type('text/html'); > $r->header_out( 'Content-Length', length($pageout) ); > $r->send_http_header(); > > my $start = 0; > my $len = 63000; > while (my $p = substr($pageout, $start, $len)) { > $start += $len; > $r->print($p); > } > $r->rflush(); >} > > ># Open content page, and check for options ># checks for tags in format: %%TAG=VALUE%% > >sub pullpage { > my $file = shift; > my ($content, $title, $template); > $content = readfile( $file ); > > while ($content =~ m/%%(.*?)=(.*?)%%/) { > my $key = $1; > my $value = $2; > SWITCH: for ($key) { > /TEMPLATE/ && do { > # Override default template > logit("Found $key - $value",2); > $template = $value; > $content =~ s/%%$key=$value%%//g; > last SWITCH; > }; > /TITLE/ && do { > logit("Found $key - $value",2); > $title = $value; > $content =~ s/%%$key=$value%%//g; > last SWITCH; > }; > /INCLUDE/ && do { > #Read in an Included file > logit("Found $key - $value",2); > my $repl = readfile( $value ); > $content =~ s/%%$key=$value%%/$repl/g; > last SWITCH; > }; > }; > } > return ($content, $title, $template); >} > > ># Reads a file and returns the content > >sub readfile { > my $file = shift; > my $rv; > logit("Opening file $file",2); > open( FILE, $file ) || return "Could not find file $file"; > my @lines = ; > close FILE || return "Could not close filehandle"; > logit("Closed file $file",2); > for (@lines) { > $rv .= $_; > } > return $rv; >} > >sub logit { > my $warning = shift; > my $level = shift || 1; > my $caller = (caller(1))[3]; > if ($debug >= $level) { > warn "$caller:\t$warning"; > } >} > >1; > > > >-- >To unsubscribe, e-mail: [EMAIL PROTECTED] >For additional commands, e-mail: [EMAIL PROTECTED] __ Gunthe
Is this secure
I've been asked if this script is secure. I believe it is. Can anyone find any problems with it? #!/usr/bin/perl -w -t use strict; use Apache; $ENV{GATEWAY_INTERFACE} =~ /^CGI-Perl/ or die "GATEWAY_INTERFACE not Perl!"; my $r = Apache->request(); my %args = $r->args(); my $path = $r->uri; ### $path =~ s/\/(.*?)$//; # Strip off the scriptname my $tmplpath = "template/"; # Setup the template path my $cont_ext = ".html"; # Allow only content files with this extension my $tmpl_ext = ".tmpl"; # Allow only template files with this extension my $template = $tmplpath ."mcti". $tmpl_ext; # Setup the template path my $page = $args{page} || "index"; # Are we requesting a page or root? my $title = "Microdyne"; # Default Title of not specified in page my $debug = 1; ### my ($content, $pageout, $newtitle, $newtmpl); ($content, $newtitle, $newtmpl) = pullpage( $page . $cont_ext ); if ($newtitle) {$title = $newtitle;} if ($newtmpl) {$template = $tmplpath . $newtmpl . $tmpl_ext;} $pageout = readfile( $template ); $pageout =~ s/%%TITLE%%/$title/g; $pageout =~ s/%%CONTENT%%/$content/g; pageout($pageout); # Spit out the content sub pageout { my $pageout = shift; $r->content_type('text/html'); $r->header_out( 'Content-Length', length($pageout) ); $r->send_http_header(); my $start = 0; my $len = 63000; while (my $p = substr($pageout, $start, $len)) { $start += $len; $r->print($p); } $r->rflush(); } # Open content page, and check for options # checks for tags in format: %%TAG=VALUE%% sub pullpage { my $file = shift; my ($content, $title, $template); $content = readfile( $file ); while ($content =~ m/%%(.*?)=(.*?)%%/) { my $key = $1; my $value = $2; SWITCH: for ($key) { /TEMPLATE/ && do { # Override default template logit("Found $key - $value",2); $template = $value; $content =~ s/%%$key=$value%%//g; last SWITCH; }; /TITLE/ && do { logit("Found $key - $value",2); $title = $value; $content =~ s/%%$key=$value%%//g; last SWITCH; }; /INCLUDE/ && do { #Read in an Included file logit("Found $key - $value",2); my $repl = readfile( $value ); $content =~ s/%%$key=$value%%/$repl/g; last SWITCH; }; }; } return ($content, $title, $template); } # Reads a file and returns the content sub readfile { my $file = shift; my $rv; logit("Opening file $file",2); open( FILE, $file ) || return "Could not find file $file"; my @lines = ; close FILE || return "Could not close filehandle"; logit("Closed file $file",2); for (@lines) { $rv .= $_; } return $rv; } sub logit { my $warning = shift; my $level = shift || 1; my $caller = (caller(1))[3]; if ($debug >= $level) { warn "$caller:\t$warning"; } } 1; -- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]