summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormkanat%bugzilla.org <>2009-09-11 18:10:13 +0200
committermkanat%bugzilla.org <>2009-09-11 18:10:13 +0200
commit7380ea9ae11764633a4b6e64850da2d84b2aaeb2 (patch)
treeab2e70e86df3d8ed53a09032777e6168d674cc6c
parent7fda8c351dd6c9621d85c9b29c5c6baa2f1eaba3 (diff)
downloadbugzilla-7380ea9ae11764633a4b6e64850da2d84b2aaeb2.tar.gz
bugzilla-7380ea9ae11764633a4b6e64850da2d84b2aaeb2.tar.xz
Bug 515191: [SECURITY] SQL Injection via Bug.search (CVE-2009-3125) and Bug.create (CVE-2009-3165)
Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=mkanat
-rw-r--r--Bugzilla/Object.pm28
-rw-r--r--Bugzilla/WebService/Bug.pm1
-rw-r--r--Bugzilla/WebService/Constants.pm2
-rw-r--r--template/en/default/global/code-error.html.tmpl5
4 files changed, 35 insertions, 1 deletions
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index bb8b45d76..456888b38 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -169,7 +169,19 @@ sub match {
next if $field eq 'OFFSET';
if ( $field eq 'LIMIT' ) {
next unless defined $value;
- $postamble = $dbh->sql_limit( $value, $criteria->{OFFSET} );
+ detaint_natural($value)
+ or ThrowCodeError('param_must_be_numeric',
+ { param => 'LIMIT',
+ function => "${class}::match" });
+ my $offset;
+ if (defined $criteria->{OFFSET}) {
+ $offset = $criteria->{OFFSET};
+ detaint_signed($offset)
+ or ThrowCodeError('param_must_be_numeric',
+ { param => 'OFFSET',
+ function => "${class}::match" });
+ }
+ $postamble = $dbh->sql_limit($value, $offset);
next;
}
elsif ( $field eq 'WHERE' ) {
@@ -185,6 +197,8 @@ sub match {
next;
}
+ $class->_check_field($field, 'match');
+
if (ref $value eq 'ARRAY') {
# IN () is invalid SQL, and if we have an empty list
# to match against, we're just returning an empty
@@ -364,6 +378,17 @@ sub create {
return $object;
}
+# Used to validate that a field name is in fact a valid column in the
+# current table before inserting it into SQL.
+sub _check_field {
+ my ($invocant, $field, $function) = @_;
+ my $class = ref($invocant) || $invocant;
+ if (!Bugzilla->dbh->bz_column_info($class->DB_TABLE, $field)) {
+ ThrowCodeError('param_invalid', { param => $field,
+ function => "${class}::$function" });
+ }
+}
+
sub check_required_create_fields {
my ($class, $params) = @_;
@@ -406,6 +431,7 @@ sub insert_create_data {
my (@field_names, @values);
while (my ($field, $value) = each %$field_values) {
+ $class->_check_field($field, 'create');
push(@field_names, $field);
push(@values, $value);
}
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index c6d620976..44382e79f 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -258,6 +258,7 @@ sub search {
}
$params = _map_fields($params);
+ delete $params->{WHERE};
# Do special search types for certain fields.
if ( my $bug_when = delete $params->{delta_ts} ) {
diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm
index bdfe24f0a..7fd7e2ae8 100644
--- a/Bugzilla/WebService/Constants.pm
+++ b/Bugzilla/WebService/Constants.pm
@@ -53,7 +53,9 @@ use constant WS_ERROR_CODE => {
param_required => 50,
params_required => 50,
object_does_not_exist => 51,
+ param_must_be_numeric => 52,
xmlrpc_invalid_value => 52,
+ param_invalid => 53,
# Bug errors usually occupy the 100-200 range.
improper_bug_id_field_value => 100,
bug_id_does_not_exist => 101,
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index e96e6d48e..4ba5e647a 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -338,6 +338,11 @@
There is no valid transition from
[%+ get_status("UNCONFIRMED") FILTER html %] to an open state.
+ [% ELSIF error == "param_invalid" %]
+ [% title = "Invalid Parameter" %]
+ <code>[% param FILTER html %]</code> is not a valid parameter
+ for the [% function FILTER html %] function.
+
[% ELSIF error == "param_must_be_numeric" %]
[% title = "Invalid Parameter" %]
Invalid parameter <code>[% param FILTER html %]</code> passed to