Gossamer Forum: Products: DBMan: Discussions: 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 }";

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");

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.