Gossamer Forum
Home : Products : DBMan : Discussions :

Security hole fix, Take 2

Quote Reply
Security hole fix, Take 2
 
There is a hole in the DBMan's db.cgi allowing anyone to execute any command on the web server Shocked. This same hole is also in Links.

As you might have learned, my previous fix introduced a bug. I do not have a webserver, so I hadn't tested my previous fix. The same goes for this one. However, there's a program below that servers as a "proof of concept". The program demonstrates that a fix is possible and that my method works.

If you try this, let us know if it works.

original:
Code:
foreach $field (@search_fields) {
my $tmpreg = "$in{$db_cols[$field]}";
(!$in{'re'}) and ($tmpreg = "\Q$tmpreg\E");
($in{'ww'}) and ($tmpreg = "\\b$tmpreg\\b");
(!$in{'cs'}) and ($tmpreg = "(?i)$tmpreg");
($in{$db_cols[$field]} eq "*") and ($tmpreg = ".*"); # A "*" matches anything.

$regexp_func[$field] = eval "sub { m/$tmpreg/o }";
$regexp_bold[$field] = $tmpreg;
}
fixed:
Code:
foreach $field (@search_fields) {
my $tmpreg = "$in{$db_cols[$field]}";
(!$in{'re'}) and ($tmpreg = "\Q$tmpreg\E");
($in{'ww'}) and ($tmpreg = "\\b$tmpreg\\b");
(!$in{'cs'}) and ($tmpreg = "(?i)$tmpreg");
($in{$db_cols[$field]} eq "*") and ($tmpreg = ".*"); # A "*" matches anything.

$tmpreg =~ s#((?<!\\)(?:\\\\)*)/#\\$1/#g; # FIX SECURITY HOLE!

$regexp_func[$field] = eval "sub { m/$tmpreg/o }";
$regexp_bold[$field] = $tmpreg;
}
Warning:

That fix may not fully block the security hole, but I think it does.


Guaranteed Fix #1

Changing
Code:
eval "sub { m/$tmpreg/o }";
to
Code:
sub { m/$tmpreg/ };
is guaranteed to fix it, but it's very very inefficient as the script recompiles the regular expression for each line (because the "o" has to be removed).


Guaranteed Fix #2

Another guaranteed way is to disable regular expression queries by changing the line
Code:
(!$in{'re'}) and ($tmpreg = "\Q$tmpreg\E");
to
Code:
$tmpreg = "\Q$tmpreg\E";
If you do this, don't forget to remove the option from the HTML page.


"Proof of concept" program:
Code:
#!/usr/local/bin/perl -w

use 5.004; # (Probably) requires perl 5.004+
use strict;

use vars qw($FIX);


$FIX = 1; # 0 = original, 1 = previously posted fix, 2 = this fix


sub make_re_func {
my $tmpreg = $_[0]; # Must be a local "my" variable.

if ($FIX == 1) {
return sub { m/$tmpreg/o };
}

elsif ($FIX == 2) {
if ($tmpreg =~ s#((?<!\\)(?:\\\\)*)/#\\$1/#g) {
warn("Fixed ${_[0]} => ${tmpreg}\n");
}
return eval "sub { m/$tmpreg/o }";
}

else {
return eval "sub { m/$tmpreg/o }";
}
}


my @test;
my $test;

@test = (
{ # Test multiple field query.
'@regexp' => ['aa', 'ab'],
'@line' => ['abbc', 'aabc'],
},

{ # Test vulnerability.
'@regexp' => ["./; print('<<HACK>>'); m/."],
'@line' => ['a', ''],
},
);


foreach $test (@test) {
my @regexp;
my @re_func;
my @line;

my $regexp;
my $re_func;
my $line;

my $q;

@regexp = @{$test->{'@regexp'}};
@line = @{$test->{'@line' }};

@re_func = ();
foreach $regexp (@regexp) {
push(@re_func, make_re_func($regexp));
}

foreach $line (@line) {
for ($q=0; $q<scalar(@re_func); $q++) {
$_ = $line;
printf("'%s' %s '%s'\n",
$regexp[$q],
&{$re_func[$q]}() ? "matches " : "doesn't match",
$line,
);
}
}

print("\n");
}

exit(0);

# With $FIX = 0 <- Original db.cgi
# -------------
# 'aa' doesn't match 'abbc'
# 'ab' matches 'abbc' <- Would say "doesn't match" if it
# 'aa' matches 'aabc' didn't work for multiple field query.
# 'ab' matches 'aabc'
#
# <<HACK>>'./; print('<<HACK>>'); m/.' matches 'a'
# <<HACK>>'./; print('<<HACK>>'); m/.' doesn't match ''
# ^
# |
# +-- Any command can be run.

# With $FIX = 1 <- Previously posted fix
# -------------
# 'aa' doesn't match 'abbc'
# 'ab' doesn't match 'abbc' <- OOPS! Doesn't work for multiple field query.
# 'aa' matches 'aabc'
# 'ab' matches 'aabc'
#
# './; print('<<HACK>>'); m/.' doesn't match 'a'
# './; print('<<HACK>>'); m/.' doesn't match ''
# ^
# |
# +-- No longer vulnerable.

# With $FIX = 2 <- Fixed db.cgi
# -------------
# 'aa' doesn't match 'abbc'
# 'ab' matches 'abbc' <- Good, it works.
# 'aa' matches 'aabc'
# 'ab' matches 'aabc'
#
# Fixed ./; print('<<HACK>>'); m/. => .\/; print('<<HACK>>'); m\/.
# './; print('<<HACK>>'); m/.' doesn't match 'a'
# './; print('<<HACK>>'); m/.' doesn't match ''
# ^
# |
# +-- No longer vulnerable.
Subject Author Views Date
Thread Security hole fix, Take 2 ELB 2884 Jul 5, 2000, 9:49 AM
Thread Since noone replied,
ELB 2774 Jul 6, 2000, 11:51 PM
Post Re: Since noone replied,
JPDeni 2771 Jul 7, 2000, 12:47 AM