Re: Bug#606311: Acknowledgement (movabletype-opensource: Unspecified XSS and SQL injection vulnerabilities fixed in 4.35)
Ignoring files that have only changed SVN ID, removed files which
were already ignored by debian/rules (mt-static/support/dashboard/stats)
and changes which only bump the version number, we have the following
changes between MTOS 4.34 and 4.35:
lib/MT/App/Search.pm | 22 +++++++++++++++++-----
lib/MT/CMS/Tools.pm | 5 ++++-
lib/MT/Template/Context/Search.pm | 4 ++--
lib/MT/Template/ContextHandlers.pm | 26 ++++++++++++++++----------
php/extlib/ezsql/ezsql_postgres.php | 2 +-
php/lib/mtdb_base.php | 23 +++++++++++++++++++----
php/mt.php | 5 +++--
7 files changed, 62 insertions(+), 25 deletions(-)
The vulnerabilities are not described by upstream except that there is
at least one XSS fix and at least one SQL injection fix.
The changes can be summarised roughly as follows:
lib/MT/App/Search.pm | 22 +++++++++++++++++-----
Input checking
lib/MT/CMS/Tools.pm | 5 ++++-
HTML/JS escaping
lib/MT/Template/Context/Search.pm | 4 ++--
URI encoding
lib/MT/Template/ContextHandlers.pm | 26 ++++++++++++++++----------
Input checking, HTML escaping
php/extlib/ezsql/ezsql_postgres.php | 2 +-
Modifying input checking
php/lib/mtdb_base.php | 23 +++++++++++++++++++----
Modifying logic to accommodate escaping
php/mt.php | 5 +++--
Modifying input checking
Although not well documented it's clear that these changes are all
security-relevant, so I propose to upload 4.3.5 to unstable and have it
migrate to testing. I will go ahead with an upload to unstable this
evening unless someone shouts.
Patch corresponding to above diffstat attached for reference.
Still TODO: assess stable.
--
Dominic Hargreaves | http://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)
diff -wurN MTOS-4.34-en//lib/MT/App/Search.pm MTOS-4.35-en//lib/MT/App/Search.pm
--- MTOS-4.34-en//lib/MT/App/Search.pm 2009-12-17 08:45:12.000000000 +0000
+++ MTOS-4.35-en//lib/MT/App/Search.pm 2010-11-25 09:04:37.000000000 +0000
@@ -670,13 +670,14 @@
$ctx->var('datebased_archive', 1) if ($app->param('archive_type') &&
( $app->param('archive_type') =~ /Daily/i || $app->param('archive_type') =~ /Weekly/i
|| $app->param('archive_type') =~ /Monthly/i || $app->param('archive_type') =~ /Yearly/i ));
- if ($app->param('author')) {
+ if ($app->param('author') && $app->param('author') =~ /^[0-9]*$/) {
require MT::Author;
- my $author = MT::Author->load($app->param('author'));
+ if ( my $author = MT::Author->load($app->param('author')) ) {
$ctx->stash('author', $author);
$ctx->var('author_archive', 1);
}
- if ($app->param('category')) {
+ }
+ if ($app->param('category') && $app->param('category') =~ /^[0-9]*$/) {
require MT::Category;
my $category = MT::Category->load($app->param('category'));
$ctx->stash('category', $category);
@@ -1004,6 +1005,8 @@
$query =~ s/'/"/g;
}
+ my $can_search_by_id = $query =~ /^[0-9]*$/ ? 1 : 0;
+
my $lucene_struct = Lucene::QueryParser::parse_query($query);
if ( 'PROHIBITED' eq $term->{type} ) {
$_->{type} = 'PROHIBITED' foreach @$lucene_struct;
@@ -1011,7 +1014,11 @@
# search for exact match
my ($terms)
- = $app->_query_parse_core( $lucene_struct, { id => 1, label => 1 }, {} );
+ = $app->_query_parse_core( $lucene_struct, {
+ ( $can_search_by_id ? ( id => 1 ) : () ),
+ label => 1
+ },
+ {} );
return unless $terms && @$terms;
push @$terms, '-and',
{
@@ -1039,12 +1046,17 @@
$query =~ s/'/"/g;
}
+ my $can_search_by_id = $query =~ /^[0-9]*$/ ? 1 : 0;
+
my $lucene_struct = Lucene::QueryParser::parse_query($query);
if ( 'PROHIBITED' eq $term->{type} ) {
$_->{type} = 'PROHIBITED' foreach @$lucene_struct;
}
my ($terms)
- = $app->_query_parse_core( $lucene_struct, { id => 1, nickname => 'like' },
+ = $app->_query_parse_core( $lucene_struct, {
+ ( $can_search_by_id ? ( id => 1 ) : () ),
+ nickname => 'like',
+ },
{} );
return unless $terms && @$terms;
push @$terms, '-and', { id => \'= entry_author_id', };
diff -wurN MTOS-4.34-en//lib/MT/CMS/Tools.pm MTOS-4.35-en//lib/MT/CMS/Tools.pm
--- MTOS-4.34-en//lib/MT/CMS/Tools.pm 2009-12-16 22:59:13.000000000 +0000
+++ MTOS-4.35-en//lib/MT/CMS/Tools.pm 2010-11-24 06:26:40.000000000 +0000
@@ -112,6 +112,9 @@
$param ||= {};
$param->{'email'} = $app->param('email');
$param->{'return_to'} = $app->param('return_to') || $cfg->ReturnToURL || '';
+ if ( $param->{recovered} ) {
+ $param->{return_to} = MT::Util::encode_js($param->{return_to});
+ }
$param->{'can_signin'} = (ref $app eq 'MT::App::CMS') ? 1 : 0;
$app->add_breadcrumb( $app->translate('Password Recovery') );
@@ -298,7 +301,7 @@
}
$app->make_commenter_session($user);
if ($redirect) {
- return $app->redirect($redirect);
+ return $app->redirect(MT::Util::encode_html($redirect));
} else {
return $app->redirect_to_edit_profile();
}
diff -wurN MTOS-4.34-en//lib/MT/Template/Context/Search.pm MTOS-4.35-en//lib/MT/Template/Context/Search.pm
--- MTOS-4.34-en//lib/MT/Template/Context/Search.pm 2009-12-17 08:45:12.000000000 +0000
+++ MTOS-4.35-en//lib/MT/Template/Context/Search.pm 2010-11-22 09:59:42.000000000 +0000
@@ -384,10 +384,10 @@
$link .= "&type=$type";
}
if ( my $include_blogs = $ctx->stash('include_blogs') ) {
- $link .= "&IncludeBlogs=$include_blogs";
+ $link .= "&IncludeBlogs=" . encode_url($include_blogs);
}
elsif ( my $blog_id = $ctx->stash('blog_id') ) {
- $link .= "&blog_id=$blog_id";
+ $link .= "&blog_id=" . encode_url($blog_id);
}
if ( my $format = $ctx->stash('format') ) {
$link .= '&format=' . encode_url($format);
diff -wurN MTOS-4.34-en//lib/MT/Template/ContextHandlers.pm MTOS-4.35-en//lib/MT/Template/ContextHandlers.pm
--- MTOS-4.34-en//lib/MT/Template/ContextHandlers.pm 2010-02-16 21:15:44.000000000 +0000
+++ MTOS-4.35-en//lib/MT/Template/ContextHandlers.pm 2010-11-25 02:24:04.000000000 +0000
@@ -9037,6 +9037,8 @@
my $flag = lc $args->{flag}
or return $ctx->error(MT->translate(
'You used <$MTEntryFlag$> without a flag.' ));
+ $e->has_column($flag)
+ or return $ctx->error(MT->translate("You have an error in your '[_2]' attribute: [_1]", $flag, 'flag'));
my $v = $e->$flag();
## The logic here: when we added the convert_breaks flag, we wanted it
## to default to checked, because we added it in 2.0, and people had
@@ -11422,7 +11424,8 @@
return $ctx->_no_comment_error();
my $label = $args->{label} || $args->{text} || MT->translate('Reply');
- my $comment_author = MT::Util::encode_html( MT::Util::encode_js($comment->author) );
+ my $comment_author = MT::Util::encode_html(
+ MT::Util::encode_html( MT::Util::encode_js( $comment->author ) ), 1 );
my $onclick = sprintf( $args->{onclick} || "mtReplyCommentOnClick(%d, '%s')", $comment->id, $comment_author);
return sprintf(qq(<a title="%s" href="javascript:void(0);" onclick="$onclick">%s</a>),
@@ -16539,6 +16542,7 @@
AssetsHeader => !$i,
AssetsFooter => !defined $assets[$i+1],
});
+ return $ctx->error( $builder->errstr ) unless defined $out;
$res .= $out;
$row_count++;
$row_count = 0 if $row_count > $per_row;
@@ -17133,6 +17137,8 @@
} elsif ($prop =~ m/^image_/) {
$ret = 0;
} else {
+ $a->has_column($prop)
+ or return $ctx->error(MT->translate("You have an error in your '[_2]' attribute: [_1]", $prop, 'property'));
$ret = $a->$prop || '';
}
@@ -20051,17 +20057,17 @@
$link .= '?';
}
}
- $link .= "limit=$limit";
+ $link .= "limit=" . encode_url($limit);
#$link .= "&offset=$offset" if $offset;
- $link .= "&category=$category" if $category;
- $link .= "&author=$author" if $author;
- $link .= "&page=$page" if $page;
- $link .= "&year=$year" if $year;
- $link .= "&month=$month" if $month;
- $link .= "&day=$day" if $day;
- $link .= "&archive_type=$archive_type" if $archive_type;
- $link .= "&template_id=$template_id" if $template_id;
+ $link .= "&category=" . encode_url($category) if $category;
+ $link .= "&author=" . encode_url($author) if $author;
+ $link .= "&page=" . encode_url($page) if $page;
+ $link .= "&year=" . encode_url($year) if $year;
+ $link .= "&month=" . encode_url($month) if $month;
+ $link .= "&day=" . encode_url($day) if $day;
+ $link .= "&archive_type=" . encode_url($archive_type) if $archive_type;
+ $link .= "&template_id=" . encode_url($template_id) if $template_id;
return $link;
}
diff -wurN MTOS-4.34-en//php/extlib/ezsql/ezsql_postgres.php MTOS-4.35-en//php/extlib/ezsql/ezsql_postgres.php
--- MTOS-4.34-en//php/extlib/ezsql/ezsql_postgres.php 2009-12-16 22:59:13.000000000 +0000
+++ MTOS-4.35-en//php/extlib/ezsql/ezsql_postgres.php 2010-11-22 08:26:43.000000000 +0000
@@ -160,7 +160,7 @@
// try to find table name
- eregi ("insert *into *([^ ]+).*", $query, $regs);
+ preg_match ("/insert *into *([^ ]+).*/i", $query, $regs);
//print_r($regs);
diff -wurN MTOS-4.34-en//php/lib/mtdb_base.php MTOS-4.35-en//php/lib/mtdb_base.php
--- MTOS-4.34-en//php/lib/mtdb_base.php 2009-12-17 08:45:12.000000000 +0000
+++ MTOS-4.35-en//php/lib/mtdb_base.php 2010-11-25 10:47:57.000000000 +0000
@@ -56,7 +56,8 @@
function unserialize($data) {
if (!$this->serializer) {
require_once("MTSerialize.php");
- $this->serializer =& new MTSerialize();
+ $serializer = new MTSerialize();
+ $this->serializer =& $serializer;
}
return $this->serializer->unserialize($data);
}
@@ -66,9 +67,8 @@
parent::query($query);
}
- function &resolve_url($path, $blog_id) {
+ function &resolve_url($path, $blog_id, $build_type = 3) {
$path = preg_replace('!/$!', '', $path);
- $path = $this->escape($path);
$blog_id = intval($blog_id);
# resolve for $path -- one of:
# /path/to/file.html
@@ -90,7 +90,7 @@
and template_type != 'backup'
order by length(fileinfo_url) asc
";
- $rows = $this->get_results(sprintf($sql,$p), ARRAY_A);
+ $rows = $this->get_results(sprintf($sql,$this->escape($p)), ARRAY_A);
if ($rows) {
break;
}
@@ -100,6 +100,21 @@
$found = false;
foreach ($rows as $row) {
+ if ( !empty( $build_type ) ) {
+ if ( !is_array( $build_type ) ) {
+ $build_type_array = array( $build_type );
+ } else {
+ $build_type_array = $build_type;
+ }
+
+ $type = isset($row['templatemap_build_type']) && strlen($row['templatemap_build_type']) > 0
+ ? $row['templatemap_build_type'] : $row['template_build_type'];
+
+ if ( !in_array( $type, $build_type_array ) ) {
+ continue;
+ }
+ }
+
$fiurl = $row['fileinfo_url'];
if ($fiurl == $path) {
$found = true;
diff -wurN MTOS-4.34-en//php/mt.php MTOS-4.35-en//php/mt.php
--- MTOS-4.34-en//php/mt.php 2010-02-17 21:55:15.000000000 +0000
+++ MTOS-4.35-en//php/mt.php 2010-11-25 15:04:36.000000000 +0000
@@ -200,7 +200,7 @@
if ($fp = file($file)) {
foreach ($fp as $line) {
// search through the file
- if (!ereg('^\s*\#',$line)) {
+ if (!preg_match('/^\s*\#/',$line)) {
// ignore lines starting with the hash symbol
if (preg_match('/^\s*(\S+)\s+(.*)$/', $line, $regs)) {
$key = strtolower(trim($regs[1]));
@@ -291,7 +291,7 @@
$data = preg_split('/[\r?\n]/', $data);
foreach ($data as $line) {
// search through the file
- if (!ereg('^\s*\#',$line)) {
+ if (!preg_match('/^\s*\#/',$line)) {
// ignore lines starting with the hash symbol
if (preg_match('/^\s*(\S+)\s+(.*)$/', $line, $regs)) {
$key = strtolower(trim($regs[1]));
@@ -454,6 +454,7 @@
}
// now set the path so it may be queried
+ $path = preg_replace('/\\\\/', '\\\\\\\\', $path );
$this->request = $path;
// When we are invoked as an ErrorDocument, the parameters are
Reply to: