Yet Another Variable Expansion Problem

strfry() has asked for the wisdom of the Perl Monks concerning the following question:

Hi all. Yes, as the title states, here's another variable expansion plea. You see, a friend of mine has a hand-rolled template parser, and won't accept my advice to check CPAN and use one of the undoubtedly better tested versions.

So I figure I'm going to give him a list of examples of why his hand-rolled system is insecure. A little scare tactic to spread the word, if you will. (: I was wondering if you, as the perlmonks.org userbase, could give me a few examples of your own, as to why the following code is insecure - the more disturbing, the better:

#!/usr/bin/perl

my ($template, $F_H,$file);

$file = "file.txt";
if( open $F_H, $file )
{
    local $/ = undef;
    $template = <$F_H>;
    close($F_H);
}

$template =~ s/\[(\$[\w\[\]\{\}\'\"]*)\]/$1/eeg;
print "Content-type: text/plain\n\n";
print $template;
[download]

file.txt is filled with 'variables' that look like [$this] - I've already found that i can call lots of perlvar ($0, $ENV{HOME}) from it, as well as such niceties as [${system'ls'}] etc. Please note that I'm disregarding the lack of use strict and -w, as I can only fight one battle at a time. ):

Can anyone find anything else insecure about it?

Thanks,

strfry()

Comment on Yet Another Variable Expansion Problem Select or Download Code

Replies are listed 'Best First'.
Re: Yet Another Variable Expansion Problem by chromatic (Archbishop) on Apr 12, 2002 at 19:59 UTC
[unlink $0] would seem to solve the main problem.	[reply]
Re: Yet Another Variable Expansion Problem by extremely (Priest) on Apr 12, 2002 at 20:12 UTC
Well, if you would change the match to something like the following and have him stick all the variables in a namespace other than main:: you'd be better off than you are. The key trick is requiring a real word character after the '$'. `$template =~ s/\[\$([a-zA-Z][\w\[\]\{\}'"])\]/'$Q::'.$1/eeg;` Basically, lop off the '$' too, make sure the first character is really a letter (and that one exists, yours allows [$] which isn't real good), and stuff that string onto a variable in another namespace. If you are sure that every variable is a scalar you might just do `/\[\$([a-zA-Z]+)\]/$hash{$1}/eg;` as well. And yes, I'm ashamed that I'm on the Template-Toolkit list and still helping you do this the wrong way. =) =) ++chromatic* -- $you = new YOU; honk() if $you->love(perl)	[reply] [d/l] [select]
Re: Yet Another Variable Expansion Problem by tachyon (Chancellor) on Apr 12, 2002 at 20:55 UTC
`[${system "rm $O"}] [${system "hackme_with_trojan"}]` [download] OK so you already showed you could run a program (ls) but you can execute any 0777 program in the current path. How about we slow the script down a bit :-) `[${sleep rand 10}]` [download] Or bring the server to its knees with some malicious code `[${fork while "waiting for system to crash"}] [${s[][out of memory] while "LOL"}]` [download] cheers tachyon `[${unlink $O and die "Goodbye"}]`	[reply] [d/l] [select]
Re: Yet Another Variable Expansion Problem by Fletch (Bishop) on Apr 12, 2002 at 19:52 UTC
One shudders to think if there were ever a template processed with `[$ENV{REMOTE_USER}]` or the like.	[reply] [d/l]
Re: Yet Another Variable Expansion Problem by erikharrison (Deacon) on Apr 12, 2002 at 20:55 UTC
The big security hole here is cross site scripting. This program never checks the content sent back to the user - if this system was used to maintain, say, a guestbook, readers of that guestbook could easily be the target of nefarious Java Script planted by a previous user. Though cross - site scripting is dangerous enough (and should scare your friend) there isn't a "remote shell access" type problem here. The reason is that we can imagine lots of nasty things that could be processed in the template; but since the substitution only occurs once, it'd be hard to force the program to do something that the designer hadn't already placed inside his template file. You can't pass in a nefarious system command and expect it to be expanded and executed - the program doesn't work that way. The REAL problem with this system is not a security one, but the fact that it only barely works. Part of the problem here is that someone works hard to learn Perl and then manages to wrap their brain's around regexes, then manages to write one that is both complicated AND works. Then you come along and tell them they've screwed up, or not thought it through. Your friend should be justly proud of his solution - it's a difficult regex he's come up with. But it should be pointed out that it can't handle complex templates - what if the HTML writer wants to use a [ just somewhere in the template? Or have something span multiple lines? Or a CDATA section? Does the program seperate logic and design? Does your firend understand the neccessity of this? These are the issues that hand rolled templating systems fall into. Scare him with cross site scripting, but make sure he understands these issues too. Good Luck getting him to `use strict`! Cheers, Erik	[reply] [d/l]
Re: Yet Another Variable Expansion Problem by tachyon (Chancellor) on Apr 12, 2002 at 23:48 UTC
Lucky spaces are not allowed: `s//${s sschr 42se and s ssrf s and s sschr 45se and s ssrm s and print +}/ee` [download] cheers tachyon	[reply] [d/l]


"be consistent"
	PerlMonks