Security fixes in moodle-1.8.2.dfsg-3 (please unblock)
(Please CC me on your replies, thanks!)
Hello,
Moodle 1.8.8 was recently released and it fixes a number of security issues
which are present in the current lenny moodle package.
Attached is a debdiff of the -2 (in lenny) against -3. It fixes all of these
vulnerabilities:
* Delete unused (but vulnerable) Spellchecker plugin to htmlarea
(MSA-09-0005, CVE-2008-5153)
* Hide images of deleted users (MSA-09-0001)
* Fix user pix disclosure (MSA-09-0002)
* Fix XSS vulnerabilities in HTML blocks (MSA-09-0004)
* Fix XSS vulnerabilities in logs (MSA-09-0007)
* Fix CSRF vulnerability in forum code (MSA-09-0008)
After talking to the testing security team, I have uploaded this package to
unstable with the hope that it will be unblocked for lenny.
Cheers,
Francois
diff -u moodle-1.8.2.dfsg/debian/rules moodle-1.8.2.dfsg/debian/rules
--- moodle-1.8.2.dfsg/debian/rules
+++ moodle-1.8.2.dfsg/debian/rules
@@ -59,6 +59,7 @@
rm -f debian/moodle/usr/share/moodle/admin/delete.php
rm -f debian/moodle/usr/share/moodle/mod/wiki/ewiki/fragments/mkhuge
rm -f debian/moodle/usr/share/moodle/search/.cvsignore
+ rm -rf debian/moodle/usr/share/moodle/lib/editor/htmlarea/plugins/SpellChecker
rm -rf debian/moodle/usr/share/moodle/lib/smarty
rm -rf debian/moodle/usr/share/moodle/lib/yui
diff -u moodle-1.8.2.dfsg/debian/changelog moodle-1.8.2.dfsg/debian/changelog
--- moodle-1.8.2.dfsg/debian/changelog
+++ moodle-1.8.2.dfsg/debian/changelog
@@ -1,3 +1,15 @@
+moodle (1.8.2.dfsg-3) unstable; urgency=high
+
+ * Delete unused (but vulnerable) Spellchecker plugin to htmlarea
+ (MSA-09-0005, CVE-2008-5153)
+ * Hide images of deleted users (MSA-09-0001)
+ * Fix user pix disclosure (MSA-09-0002)
+ * Fix XSS vulnerabilities in HTML blocks (MSA-09-0004)
+ * Fix XSS vulnerabilities in logs (MSA-09-0007)
+ * Fix CSRF vulnerability in forum code (MSA-09-0008)
+
+ -- Francois Marier <francois@debian.org> Mon, 02 Feb 2009 19:09:10 +1300
+
moodle (1.8.2.dfsg-2) unstable; urgency=high
[ Dan Poltawski ]
diff -u moodle-1.8.2.dfsg/debian/patches/00list moodle-1.8.2.dfsg/debian/patches/00list
--- moodle-1.8.2.dfsg/debian/patches/00list
+++ moodle-1.8.2.dfsg/debian/patches/00list
@@ -2,0 +3,5 @@
+msa090001.dpatch
+msa090002.dpatch
+msa090004.dpatch
+msa090007.dpatch
+msa090008.dpatch
only in patch2:
unchanged:
--- moodle-1.8.2.dfsg.orig/debian/patches/msa090004.dpatch
+++ moodle-1.8.2.dfsg/debian/patches/msa090004.dpatch
@@ -0,0 +1,62 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## msa090004.dpatch by Francois Marier <francois@debian.org>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: html block: proper cleanup of html
+
+@DPATCH@
+diff --git a/blocks/html/block_html.php b/blocks/html/block_html.php
+index ff53961..7099a43 100755
+--- a/blocks/html/block_html.php
++++ b/blocks/html/block_html.php
+@@ -12,7 +12,7 @@ class block_html extends block_base {
+ }
+
+ function specialization() {
+- $this->title = isset($this->config->title) ? $this->config->title : get_string('newhtmlblock', 'block_html');
++ $this->title = isset($this->config->title) ? format_string($this->config->title) : get_string('newhtmlblock', 'block_html');
+ }
+
+ function instance_allow_multiple() {
+@@ -24,8 +24,13 @@ class block_html extends block_base {
+ return $this->content;
+ }
+
+- $filteropt = new stdClass;
+- $filteropt->noclean = true;
++ if (!empty($this->instance->pinned) or $this->instance->pagetype === 'course-view') {
++ // fancy html allowed only on course page and in pinned blocks for security reasons
++ $filteropt = new stdClass;
++ $filteropt->noclean = true;
++ } else {
++ $filteropt = null;
++ }
+
+ $this->content = new stdClass;
+ $this->content->text = isset($this->config->text) ? format_text($this->config->text, FORMAT_HTML, $filteropt) : '';
+diff --git a/blocks/html/config_instance.html b/blocks/html/config_instance.html
+index 8138488..ae2d460 100755
+--- a/blocks/html/config_instance.html
++++ b/blocks/html/config_instance.html
+@@ -1,4 +1,11 @@
+-<?php $usehtmleditor = can_use_html_editor(); ?>
++<?php
++ $usehtmleditor = can_use_html_editor();
++
++ $text = isset($this->config->text) ? $this->config->text : '';
++ if (empty($this->instance->pinned) and $this->instance->pagetype !== 'course-view') {
++ $text = clean_text($text, FORMAT_HTML);
++ }
++?>
+ <table cellpadding="9" cellspacing="0">
+ <tr valign="top">
+ <td align="right"><?php print_string('configtitle', 'block_html'); ?>:</td>
+@@ -6,7 +13,7 @@
+ </tr>
+ <tr valign="top">
+ <td align="right"><?php print_string('configcontent', 'block_html'); ?>:</td>
+- <td><?php print_textarea($usehtmleditor, 25, 50, 0, 0, 'text', isset($this->config->text)?$this->config->text:'') ?></td>
++ <td><?php print_textarea($usehtmleditor, 25, 50, 0, 0, 'text', $text) ?></td>
+ </tr>
+ <tr>
+ <td colspan="3" align="center">
only in patch2:
unchanged:
--- moodle-1.8.2.dfsg.orig/debian/patches/msa090008.dpatch
+++ moodle-1.8.2.dfsg/debian/patches/msa090008.dpatch
@@ -0,0 +1,63 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## msa090008.dpatch by Francois Marier <francois@debian.org>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: forum: add sesskey to post/discussion deletion
+
+@DPATCH@
+diff --git a/mod/forum/post.php b/mod/forum/post.php
+index 8a4760b..cf06da1 100644
+--- a/mod/forum/post.php
++++ b/mod/forum/post.php
+@@ -266,7 +266,7 @@
+
+ $replycount = forum_count_replies($post);
+
+- if (!empty($confirm)) { // User has confirmed the delete
++ if (!empty($confirm) && confirm_sesskey()) { // User has confirmed the delete
+
+ if ($post->totalscore) {
+ notice(get_string("couldnotdeleteratings", "forum"),
+@@ -320,7 +320,7 @@
+ }
+ print_header();
+ notice_yesno(get_string("deletesureplural", "forum", $replycount+1),
+- "post.php?delete=$delete&confirm=$delete",
++ "post.php?delete=$delete&confirm=$delete&sesskey=".sesskey(),
+ $CFG->wwwroot.'/mod/forum/discuss.php?d='.$post->discussion.'#p'.$post->id);
+
+ forum_print_post($post, $course->id, $ownpost=false, $reply=false, $link=false);
+@@ -335,7 +335,7 @@
+ } else {
+ print_header();
+ notice_yesno(get_string("deletesure", "forum", $replycount),
+- "post.php?delete=$delete&confirm=$delete",
++ "post.php?delete=$delete&confirm=$delete&sesskey=".sesskey(),
+ $CFG->wwwroot.'/mod/forum/discuss.php?d='.$post->discussion.'#p'.$post->id);
+ forum_print_post($post, $forum->course, $ownpost=false, $reply=false, $link=false);
+ }
+diff --git a/mod/forum/post.php b/mod/forum/post.php
+index cf06da1..83fb15a 100644
+--- a/mod/forum/post.php
++++ b/mod/forum/post.php
+@@ -371,7 +371,7 @@
+ error("You can't split discussions!");
+ }
+
+- if (!empty($name)) { // User has confirmed the prune
++ if (!empty($name) && confirm_sesskey()) { // User has confirmed the prune
+
+ $newdiscussion = new object();
+ $newdiscussion->course = $discussion->course;
+diff --git a/mod/forum/prune.html b/mod/forum/prune.html
+index 383b3a9..1a4f4b7 100644
+--- a/mod/forum/prune.html
++++ b/mod/forum/prune.html
+@@ -11,6 +11,7 @@
+ <td align="center" colspan="2">
+ <input type="hidden" name="prune" value="<?php p($prune) ?>" />
+ <input type="hidden" name="confirm" value="<?php p($prune) ?>" />
++ <input type="hidden" name="sesskey" value="<?php echo sesskey() ?>" />
+ <input type="submit" value="<?php print_string('prune', 'forum'); ?>" />
+ </td>
+ </tr>
only in patch2:
unchanged:
--- moodle-1.8.2.dfsg.orig/debian/patches/msa090001.dpatch
+++ moodle-1.8.2.dfsg/debian/patches/msa090001.dpatch
@@ -0,0 +1,29 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## msa090001.dpatch by Francois Marier <francois@debian.org>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: user: profile images of deleted users are not accessible anymore
+
+@DPATCH@
+diff --git a/user/pix.php b/user/pix.php
+index 8eb3c09..b3509ef 100644
+--- a/user/pix.php
++++ b/user/pix.php
+@@ -17,10 +17,13 @@
+
+ if (count($args) == 2) {
+ $userid = (integer)$args[0];
+- $image = $args[1];
+- $pathname = $CFG->dataroot.'/users/'.$userid.'/'.$image;
+- if (file_exists($pathname) and !is_dir($pathname)) {
+- send_file($pathname, $image);
++ // do not serve images of deleted users
++ if ($user = get_record('user', 'id', $userid, 'deleted', 0, 'picture', 1)) {
++ $image = $args[1];
++ $pathname = $CFG->dataroot.'/users/'.$userid.'/'.$image;
++ if (file_exists($pathname) and !is_dir($pathname)) {
++ send_file($pathname, $image);
++ }
+ }
+ }
+
only in patch2:
unchanged:
--- moodle-1.8.2.dfsg.orig/debian/patches/msa090007.dpatch
+++ moodle-1.8.2.dfsg/debian/patches/msa090007.dpatch
@@ -0,0 +1,99 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## msa090007.dpatch by Francois Marier <francois@debian.org>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: proper log url sanitisation
+
+@DPATCH@
+diff --git a/course/lib.php b/course/lib.php
+index c8c36d2..031d39f 100644
+--- a/course/lib.php
++++ b/course/lib.php
+@@ -244,21 +244,49 @@ function make_log_url($module, $url) {
+ case 'message':
+ case 'calendar':
+ case 'blog':
+- return "/$module/$url";
++ if (strpos($url, '../') === 0) {
++ $url = ltrim($url, '.');
++ } else {
++ $url = "/course/$url";
++ }
+ break;
+ case 'upload':
+- return $url;
++ $url = $url;
+ break;
+ case 'library':
+ case '':
+- return '/';
++ $url = '/';
+ break;
+ default:
+- return "/mod/$module/$url";
++ $url = "/mod/$module/$url";
+ break;
+ }
+-}
+
++ //now let's sanitise urls - there might be some ugly nasties:-(
++ $parts = explode('?', $url);
++ $script = array_shift($parts);
++ if (strpos($script, 'http') === 0) {
++ $script = clean_param($script, PARAM_URL);
++ } else {
++ $script = clean_param($script, PARAM_PATH);
++ }
++
++ $query = '';
++ if ($parts) {
++ $query = implode('', $parts);
++ $query = str_replace('&', '&', $query); // both & and & are stored in db :-|
++ $parts = explode('&', $query);
++ $eq = urlencode('=');
++ foreach ($parts as $key=>$part) {
++ $part = urlencode(urldecode($part));
++ $part = str_replace($eq, '=', $part);
++ $parts[$key] = $part;
++ }
++ $query = '?'.implode('&', $parts);
++ }
++
++ return $script.$query;
++}
+
+ function build_mnet_logs_array($hostid, $course, $user=0, $date=0, $order="l.time ASC", $limitfrom='', $limitnum='',
+ $modname="", $modid=0, $modaction="", $groupid=0) {
+@@ -504,10 +532,6 @@ function print_log($course, $user=0, $date=0, $order="l.time ASC", $page=0, $per
+ //Filter log->info
+ $log->info = format_string($log->info);
+
+- $log->url = strip_tags(urldecode($log->url)); // Some XSS protection
+- $log->info = strip_tags(urldecode($log->info)); // Some XSS protection
+- $log->url = s($log->url); /// XSS protection and XHTML compatibility - should be in link_to_popup_window() instead!!
+-
+ echo '<tr class="r'.$row.'">';
+ if ($course->id == SITEID) {
+ echo "<td class=\"cell c0\">\n";
+@@ -615,10 +639,6 @@ function print_mnet_log($hostid, $course, $user=0, $date=0, $order="l.time ASC",
+ //Filter log->info
+ $log->info = format_string($log->info);
+
+- $log->url = strip_tags(urldecode($log->url)); // Some XSS protection
+- $log->info = strip_tags(urldecode($log->info)); // Some XSS protection
+- $log->url = s($log->url); /// XSS protection and XHTML compatibility - should be in link_to_popup_window() instead!!
+-
+ echo '<tr class="r'.$row.'">';
+ if ($course->id == SITEID) {
+ echo "<td class=\"r$row c0\" >\n";
+@@ -710,10 +730,7 @@ function print_log_csv($course, $user, $date, $order='l.time DESC', $modname,
+
+ //Filter log->info
+ $log->info = format_string($log->info);
+-
+- $log->url = strip_tags(urldecode($log->url)); // Some XSS protection
+ $log->info = strip_tags(urldecode($log->info)); // Some XSS protection
+- $log->url = str_replace('&', '&', $log->url); // XHTML compatibility
+
+ $firstField = $courses[$log->course];
+ $fullname = fullname($log, has_capability('moodle/site:viewfullnames', get_context_instance(CONTEXT_COURSE, $course->id)));
only in patch2:
unchanged:
--- moodle-1.8.2.dfsg.orig/debian/patches/msa090002.dpatch
+++ moodle-1.8.2.dfsg/debian/patches/msa090002.dpatch
@@ -0,0 +1,29 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## msa090002.dpatch by Francois Marier <francois@debian.org>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: protect user profile images if $CFG->forcelogin enabled
+
+@DPATCH@
+diff --git a/user/pix.php b/user/pix.php
+index b3509ef..d37d18b 100644
+--- a/user/pix.php
++++ b/user/pix.php
+@@ -3,11 +3,15 @@
+ // Syntax: pix.php/userid/f1.jpg or pix.php/userid/f2.jpg
+ // OR: ?file=userid/f1.jpg or ?file=userid/f2.jpg
+
+- $nomoodlecookie = true; // Because it interferes with caching
+-
+ require_once('../config.php');
+ require_once($CFG->libdir.'/filelib.php');
+
++ if (!empty($CFG->forcelogin) and !isloggedin()) {
++ // protect images if login required and not logged in;
++ // do not use require_login() because it is expensive and not suitable here anyway
++ redirect($CFG->pixpath.'/u/f1.png');
++ }
++
+ // disable moodle specific debug messages
+ disable_debugging();
+
Reply to: