Serious holes affecting SiteBar 3.3.8 Oct 18 2007 08:01PM
Tim Brown (timb nth-dimension org uk)
All,

As a result of a short security audit of SiteBar, a number of security holes
were found. The holes included code execution, a malicious redirect and
multiple cases of Javascript injection.

After liasing with the developers, the holes have been patched. Attached are
the advisory and patch relating to these flaws.

CVEs open already relating to this audit:

* CVE-2006-3320 (Javascript injection) - previously reported by other parties
but not resolved and so included for completeness

* CVE-2007-5492 (code execution) - first reported in my attached advisory to
the vendor, independently rediscovered by Robert Buchholz of Gentoo whilst
auditing the differences between the patched and unpatched versions (3.3.8 vs
3.3.9)

* CVE-2007-5491 (file permissions issue) - apparently patched by the vendor at
the same time as my issues were resolved and discovered by Robert Buchholz of
Gentoo whilst auditing the differences between the patched and unpatched
versions (3.3.8 vs 3.3.9)

It is intended that CVE-2007-5492 will be updated to reference both code
execution flaws I reported. All other issues in the advisory have been
patched but no CVEs have yet been requested or assigned to the best of my
knowledge.

Tim
--
Tim Brown
<mailto:timb (at) nth-dimension.org (dot) uk [email concealed]>
<http://www.nth-dimension.org.uk/>
Index: command.php
===================================================================
--- command.php (revision 412)
+++ command.php (working copy)
@@ -94,8 +94,15 @@
{
if (!$this->um->isAuthorized($this->command,
in_array($this->command, array('Log In', 'Log Out', 'Sign Up')),
- SB_reqVal('command_gid'), SB_reqVal('nid_acl'), SB_reqVal('lid_acl')))
+ SB_reqValInt('command_gid'), SB_reqValInt('nid_acl'), SB_reqValInt('lid_acl')))
{
+ $bld = 'build' . $this->shortName();
+ $cmd = 'command' . $this->shortName();
+
+ if (!method_exists($this,$bld) && !method_exists($this,$cmd))
+ {
+ $this->command = 'Unknown command!';
+ }
$this->um->accessDenied();
return;
}
@@ -849,6 +856,7 @@
// be otherwise lost. Needed to go back.
if ($disabled && $params['type'] == 'text')
{
+ $params['value'] = str_replace('"',"'",$params['value']);
?>
<input type="hidden" name="<?php echo SB_safeVal($params,'name') ?>" value="<?php echo $params['value']?>">
<?php
@@ -857,6 +865,7 @@

if ($name{0} == '-')
{
+ $params['value'] = str_replace('"',"'",$params['value']);
?>
<input type="hidden" name="<?php echo $params['name']?>" value="<?php echo $params['value']?>">
<?php
@@ -927,7 +936,7 @@
}
elseif (isset($params['type']) && ($params['type'] == 'button') || ($params['type'] == 'addbutton'))
{
- if (!$this->um->isAuthorized($name,false,null,SB_reqVal('nid_acl'),SB_reqVa
l('lid_acl'))) continue;
+ if (!$this->um->isAuthorized($name,false,null,SB_reqValInt('nid_acl'),SB_re
qValInt('lid_acl'))) continue;

if ($params['type'] == 'button')
{
@@ -1664,7 +1673,7 @@

function buildDeleteTree()
{
- $node = $this->tree->getNode(SB_reqVal('nid_acl',true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl',true));
if (!$node) return null;

$fields['Folder Name'] = array('name'=>'name','value'=>$node->name, 'disabled'=>null);
@@ -1677,10 +1686,10 @@

function commandDeleteTree()
{
- $this->tree->removeNode(SB_reqVal('nid_acl'), false);
+ $this->tree->removeNode(SB_reqValInt('nid_acl'), false);
if ($this->um->getParam('user','use_trash'))
{
- $this->tree->purgeNode(SB_reqVal('nid_acl'));
+ $this->tree->purgeNode(SB_reqValInt('nid_acl'));
}
SB_unsetVal('nid_acl');
$this->forwardCommand('Maintain Trees');
@@ -1834,7 +1843,8 @@
return;
}

- if (SB_reqChk('forward'))
+ // This should handle login from translator.php, we should avoid external redirect
+ if (SB_reqChk('forward') && strpos(SB_reqVal('forward'),'/') === false)
{
header('Location: '.SB_reqVal('forward'));
exit;
@@ -2681,14 +2691,14 @@
return null;
}

- if (SB_reqVal('uid') == SB_ADMIN)
+ $uid = intval(SB_reqVal('uid'));
+
+ if ($uid == SB_ADMIN)
{
$this->error('Cannot modify administrator!');
return null;
}

- $uid = SB_reqVal('uid');
-
$fields = array();
$user = $this->um->getUser($uid);
$fields['Username'] = array('name'=>'email', 'value'=>$user['username'], 'disabled' => null);
@@ -3960,7 +3970,7 @@
function buildAddFolder()
{
$fields = array();
- $node = $this->tree->getNode(SB_reqVal('nid_acl',true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl',true));
if (!$node) return null;

if ($this->command == 'Add Folder')
@@ -4020,7 +4030,7 @@

function commandAddFolder()
{
- $nid = $this->tree->addNode(SB_reqVal('nid_acl'),SB_reqVal('name'),
+ $nid = $this->tree->addNode(SB_reqValInt('nid_acl'),SB_reqVal('name'),
SB_reqVal('comment'), SB_reqVal('sort_mode'));

if ($this->um->pmode && !$this->hasErrors())
@@ -4037,7 +4047,7 @@
$this->skipBuild = true;
$this->reload = !$this->um->getParam('user','extern_commander');
$this->close = $this->um->getParam('user','auto_close');
- $this->um->hiddenFolders[SB_reqVal('nid_acl')] = 1;
+ $this->um->hiddenFolders[SB_reqValInt('nid_acl')] = 1;
$this->um->setParam('user','hidden_folders', implode(':',array_keys($this->um->hiddenFolders)));
$this->um->saveUserParams();
}
@@ -4048,7 +4058,7 @@
$this->reload = !$this->um->getParam('user','extern_commander');
$this->close = $this->um->getParam('user','auto_close');

- $parent = $this->tree->getNode(SB_reqVal('nid_acl'));
+ $parent = $this->tree->getNode(SB_reqValInt('nid_acl'));

$this->tree->loadNodes($parent, false, 'select', true);

@@ -4073,7 +4083,7 @@

function buildFolderProperties()
{
- $node = $this->tree->getNode(SB_reqVal('nid_acl', true));
+ $node = $this->tree->getNode( intval(SB_reqValInt('nid_acl', true)) );

$fields = $this->buildAddFolder();

@@ -4100,13 +4110,13 @@

function commandFolderProperties()
{
- $node = $this->tree->getNode(SB_reqVal('nid_acl', true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl', true));
if ($node->id_parent && !$node->parentHasRight('update'))
{
return;
}

- $nid = SB_reqVal('nid_acl');
+ $nid = SB_reqValInt('nid_acl');

$columns = array
(
@@ -4131,7 +4141,7 @@

function buildCustomOrder()
{
- $node = $this->tree->getNode(SB_reqVal('nid_acl', true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl', true));
$this->tree->loadNodes($node);

$fields['-raw1-'] = "<table cellpadding='0'>";
@@ -4155,7 +4165,7 @@

function commandCustomOrder()
{
- $node = $this->tree->getNode(SB_reqVal('nid_acl', true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl', true));
$this->tree->loadNodes($node);

$order = array();
@@ -4196,7 +4206,7 @@
$fields['Delete Content Only'] = array('name'=>'content','type'=>'checkbox',
'title'=>SB_P('command::tooltip_delete_content'));

- $node = $this->tree->getNode(SB_reqVal('nid_acl', true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl', true));

if ($this->_deleteContentOnly($node))
{
@@ -4209,14 +4219,14 @@

function commandDeleteFolder()
{
- $node = $this->tree->getNode(SB_reqVal('nid_acl', true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl', true));
$deleteContentOnly = SB_reqVal('content') || $this->_deleteContentOnly($node);

- $this->tree->removeNode(SB_reqVal('nid_acl'), $deleteContentOnly);
+ $this->tree->removeNode(SB_reqValInt('nid_acl'), $deleteContentOnly);

if (!$this->um->getParam('user','use_trash') && $node->hasRight('purge'))
{
- $this->tree->purgeNode(SB_reqVal('nid_acl'));
+ $this->tree->purgeNode(SB_reqValInt('nid_acl'));
}
}

@@ -4229,7 +4239,7 @@

function commandPurgeFolder()
{
- $this->tree->purgeNode(SB_reqVal('nid_acl'));
+ $this->tree->purgeNode(SB_reqValInt('nid_acl'));
}

/***********************************************************************
*******/
@@ -4241,7 +4251,7 @@

function commandUndelete()
{
- $this->tree->undeleteNode(SB_reqVal('nid_acl'));
+ $this->tree->undeleteNode(SB_reqValInt('nid_acl'));
}

/***********************************************************************
*******/
@@ -4261,7 +4271,7 @@
$sourceId = SB_reqVal('sid',true);
$sourceIsNode = SB_reqVal('stype',true);
$sourceObj = null;
- $targetID = SB_reqVal('nid_acl',true);
+ $targetID = SB_reqValInt('nid_acl',true);
$targetNode = $this->tree->getNode($targetID);
$sourceNodeId = $sourceId;

@@ -4337,7 +4347,7 @@

function commandPaste()
{
- $targetID = SB_reqVal('nid_acl');
+ $targetID = SB_reqValInt('nid_acl');
$sourceId = SB_reqVal('sid',true);
$sourceIsNode = SB_reqVal('stype',true);
$move = SB_reqVal('mode',true)=='Move';
@@ -4401,10 +4411,10 @@
function buildEmailLink()
{
$fields = array();
- $link = $this->tree->getLink(SB_reqVal('lid_acl'));
+ $link = $this->tree->getLink(SB_reqValInt('lid_acl'));
if (!$link) return null;

- $fields['--hidden1--'] = array('name'=>'lid_acl', 'value'=> SB_reqVal('lid_acl'));
+ $fields['--hidden1--'] = array('name'=>'lid_acl', 'value'=> SB_reqValInt('lid_acl'));

if ($this->um->canUseMail())
{
@@ -4433,7 +4443,7 @@
return;
}

- $link = $this->tree->getLink(SB_reqVal('lid_acl'));
+ $link = $this->tree->getLink(SB_reqValInt('lid_acl'));
if (!$link) return null;

$subject = SB_T('SiteBar: Web site') . ' ' . $link->name;
@@ -4520,7 +4530,7 @@

if (SB_reqChk('nid_acl') && SB_reqVal('bookmarklet')!=1)
{
- $node = $this->tree->getNode(SB_reqVal('nid_acl'));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl'));
$fields['-hidden0-'] = array('name'=>'nid_acl','value'=>$node->id);
$fields['Parent Folder'] = array('name'=>'parent',
'value'=>$node->name,'disabled'=>null);
@@ -4604,7 +4614,7 @@

function commandAddLink()
{
- $nid = SB_reqVal('nid_acl',true);
+ $nid = SB_reqValInt('nid_acl',true);
$node = $this->tree->getNode($nid);
if (!$node) return;

@@ -4639,7 +4649,7 @@
if (!$page->isDead && $page->errorCode['FAVURL']<PP_ERR)
{
$favicon = $page->info['FAVURL'];
- $favurl = 'favicon.php?' . md5($favicon) . '=' . SB_reqVal('lid_acl');
+ $favurl = 'favicon.php?' . md5($favicon) . '=' . SB_reqValInt('lid_acl');
$this->message = SB_T('Favicon <img src="%s"> found at url %s.', array($favurl, $url));
}
else
@@ -4675,7 +4685,7 @@

function commandMarkasDefault()
{
- $this->um->setParam('user','default_folder',SB_reqVal('nid_acl'));
+ $this->um->setParam('user','default_folder',SB_reqValInt('nid_acl'));
$this->um->saveUserParams();
exit;
}
@@ -4712,7 +4722,7 @@

if ($this->command!='Add Link')
{
- $link = $this->tree->getLink(SB_reqVal('lid_acl'));
+ $link = $this->tree->getLink(SB_reqValInt('lid_acl'));
if (!$link) return null;
}
else
@@ -4805,7 +4815,7 @@
}
else
{
- $fields['-raw2-'] = $this->_buildFavicon(SB_reqVal('lid_acl'), $link->favicon);
+ $fields['-raw2-'] = $this->_buildFavicon(SB_reqValInt('lid_acl'), $link->favicon);
}
}
}
@@ -4910,7 +4920,7 @@
{
if (SB_reqVal('private'))
{
- $link = $this->tree->getLink(SB_reqVal('lid_acl'));
+ $link = $this->tree->getLink(SB_reqValInt('lid_acl'));
if (!$link) return;
if (!$this->tree->inMyTree($link->id_parent))
{
@@ -4941,7 +4951,7 @@
else
{
// Delete old URL favicon from cache on update to allow new version
- $fc->purge(SB_reqVal('lid_acl'));
+ $fc->purge(SB_reqValInt('lid_acl'));
}
}

@@ -4962,13 +4972,13 @@
$update['is_dead'] = 0;
}

- $this->tree->updateLink(SB_reqVal('lid_acl', true), $update);
+ $this->tree->updateLink(SB_reqValInt('lid_acl', true), $update);
}

function buildExportDescription()
{
$fields['Decode Using'] = array('type'=>'callback', 'function'=>'_buildDecodeUsing');
- $fields['-hidden1-'] = array('name'=>'lid_acl','value'=>SB_reqVal('lid_acl'));
+ $fields['-hidden1-'] = array('name'=>'lid_acl','value'=>SB_reqValInt('lid_acl'));

return $fields;
}
@@ -4984,7 +4994,7 @@

function commandExportDescription()
{
- $link = $this->tree->getLink(SB_reqVal('lid_acl'));
+ $link = $this->tree->getLink(SB_reqValInt('lid_acl'));
if (!strlen($link->comment))
{
$this->error('Cannot export empty description!');
@@ -5019,7 +5029,7 @@
{
$fields['Description File'] = array('type'=>'file','name'=>'file');
$fields['Encode Using'] = array('type'=>'callback', 'function'=>'_buildEncodeUsing');
- $fields['-hidden1-'] = array('name'=>'lid_acl','value'=>SB_reqVal('lid_acl'));
+ $fields['-hidden1-'] = array('name'=>'lid_acl','value'=>SB_reqValInt('lid_acl'));
return $fields;
}

@@ -5039,7 +5049,7 @@
return;
}
$filename = $_FILES['file']['tmp_name'];
- $link = $this->tree->getLink(SB_reqVal('lid_acl'));
+ $link = $this->tree->getLink(SB_reqValInt('lid_acl'));

if ($this->hasErrors())
{
@@ -5109,7 +5119,7 @@

function commandDeleteLink()
{
- $link = $this->tree->getLink(SB_reqVal('lid_acl'));
+ $link = $this->tree->getLink(SB_reqValInt('lid_acl'));

if (!$link)
{
@@ -5135,7 +5145,7 @@
function buildSecurity()
{
$fields = array();
- $node = $this->tree->getNode(SB_reqVal('nid_acl',true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl',true));

$fields['Folder Name'] = array('name'=>'name','value'=>$node->name,'disabled'=>null);
$fields['Security'] = array('type'=>'callback',
@@ -5263,7 +5273,7 @@
{
$groups = $this->um->getGroups();
$myGroups = $this->um->getUserGroups();
- $node = $this->tree->getNode(SB_reqVal('nid_acl',true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl',true));
$sameACL = true;
$updated = 0;

@@ -5335,7 +5345,7 @@
function buildValidateLinks()
{
$fields = array();
- $node = $this->tree->getNode(SB_reqVal('nid_acl',true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl',true));
if (!$node) return null;

$fields['Folder Name'] = array('name'=>'name','maxlength'=>255,
@@ -5370,7 +5380,7 @@
function buildValidation()
{
$fields = array();
- $node = $this->tree->getNode(SB_reqVal('nid_acl',true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl',true));
if (!$node) return null;

require_once('./inc/validator.inc.php');
@@ -5415,7 +5425,7 @@
function buildImportBookmarks()
{
$fields = array();
- $node = $this->tree->getNode(SB_reqVal('nid_acl',true));
+ $node = $this->tree->getNode(SB_reqValInt('nid_acl',true));

$loaders['auto'] = array('', true);
$dirName = './inc/loaders';
@@ -5535,7 +5545,7 @@
'Imported %s link(s) into %s folder(s) from the bookmark file.',
array($bm->importedLinks, $bm->importedFolders));

- $this->tree->importTree(SB_reqVal('nid_acl'), $bm->root, SB_reqChk('rename'));
+ $this->tree->importTree(SB_reqValInt('nid_acl'), $bm->root, SB_reqChk('rename'));
}

function optionalExportBookmarks()
@@ -5623,7 +5633,7 @@

if (!SB_reqChk('doall'))
{
- $fields['-hidden1-'] = array('name'=>'nid_acl','value'=>SB_reqVal('nid_acl'));
+ $fields['-hidden1-'] = array('name'=>'nid_acl','value'=>SB_reqValInt('nid_acl'));
}
else
{
@@ -5681,9 +5691,9 @@
}
}

- if (SB_reqChk('nid_acl') && SB_reqVal('nid_acl')>0)
+ if (SB_reqChk('nid_acl') && SB_reqValInt('nid_acl')>0)
{
- $params[] = 'root=' . SB_reqVal('nid_acl');
+ $params[] = 'root=' . SB_reqValInt('nid_acl');
}

if (count($params))
@@ -5718,7 +5728,7 @@

if (!SB_reqChk('doall'))
{
- $fields['-hidden1-'] = array('name'=>'nid_acl','value'=>SB_reqVal('nid_acl'));
+ $fields['-hidden1-'] = array('name'=>'nid_acl','value'=>SB_reqValInt('nid_acl'));
}
else
{
Index: google.php
===================================================================
--- google.php (revision 0)
+++ google.php (revision 0)
@@ -0,0 +1,67 @@
+<?php
+
+/**********************************************************************
********
+ * SiteBar 3 - The Bookmark Server for Personal and Team Use. *
+ * Copyright (C) 2006 Ondrej Brablc <http://brablc.com/mailto?o> *
+ * *
+ * This program is free software; you can redistribute it and/or modify *
+ * it under the terms of the GNU General Public License as published by *
+ * the Free Software Foundation; either version 2 of the License, or *
+ * (at your option) any later version. *
+ * *
+ * This program is distributed in the hope that it will be useful, *
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
+ * GNU General Public License for more details. *
+ * *
+ * You should have received a copy of the GNU General Public License *
+ * along with this program; if not, write to the Free Software *
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA *
+ ************************************************************************
******/
+
+header("Content-type: text/xml");
+echo '<?xml version="1.0" encoding="UTF-8" ?>'."\n";
+?>
+<Module>
+ <ModulePrefs
+ title="SiteBar"
+ description="SiteBar Bookmark Manager"
+ author="Ondrej Brablc"
+ author_affiliation="SiteBar.org"
+ author_location="Prague, Czech Republic"
+ title_url="http://www.sitebar.org/"
+ category="communication"
+ category2="tools"
+ />
+ <Content type="html">
+ <![CDATA[
+<?php
+ $height = "400px";
+ if (isset($_GET['height']))
+ {
+ if (preg_match('/^(\d+)(.*)?$/',$_GET['height'],$reg))
+ {
+ $height = $reg[1];
+ if ($reg[2] == '%')
+ {
+ $height .= '%';
+ }
+ else
+ {
+ $height .= 'px';
+ }
+ }
+ }
+
+ require_once('./inc/errorhandler.inc.php');
+ require_once('./inc/page.inc.php');
+ require_once('./inc/usermanager.inc.php');
+
+ $um = SB_UserManager::staticInstance();
+ $url = SB_Page::absBaseUrl();
+?>
+ <iframe style="border: none; width:100%;height:<?php echo $height;?>"
+ src="<?php echo $url;?>?target=_top" />
+ ]]>
+ </Content>
+</Module>
Index: inc/database.inc.php
===================================================================
--- inc/database.inc.php (revision 412)
+++ inc/database.inc.php (working copy)
@@ -18,7 +18,7 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA *
************************************************************************
******/

-define ( 'SB_CURRENT_RELEASE', '3.3.8');
+define( 'SB_CURRENT_RELEASE', '3.3.9');

require_once('./inc/errorhandler.inc.php');

@@ -209,10 +209,12 @@

function fetchRecord($request, $binary = false)
{
+ $this->sw->cont();
$record = $this->fetchArray($request);

if (!$record)
{
+ $this->sw->pause();
return false;
}
else
@@ -221,12 +223,14 @@
{
array_walk($record, array( $this, '_unescape'));
}
+ $this->sw->pause();
return $record;
}
}

function fetchRecords($request)
{
+ $this->sw->cont();
$records = array();

while (($record = $this->fetchArray($request)))
@@ -235,6 +239,7 @@
$records[] = $record;
}

+ $this->sw->pause();
return $records;
}

Index: inc/errorhandler.inc.php
===================================================================
--- inc/errorhandler.inc.php (revision 412)
+++ inc/errorhandler.inc.php (working copy)
@@ -27,7 +27,7 @@
// Please note that the http server must have rights to write to the file
// specified bellow. You may change the log file path here.
define('SB_LOG_FILE_PATH', 'sitebar.log');
-define('SB_SHOW_PHP_ERRORS', SB_DEBUGGING);
+define('SB_SHOW_PHP_ERRORS', false);
define('SB_LOG_HTTP', SB_DEBUGGING && false);
define('SB_LOG_SQL', SB_DEBUGGING && false);

Index: inc/loaders/netscape.inc.php
===================================================================
--- inc/loaders/netscape.inc.php (revision 412)
+++ inc/loaders/netscape.inc.php (working copy)
@@ -61,7 +61,7 @@
$line = $this->toUTF8($line);

// Open node
- if (preg_match('/<DT.*><H3([^>]*)>([^<]+)<\/H3>/i', $line, $reg ))
+ if (preg_match('/<DT.*><H3([^>]*)>([^<]*)<\/H3>/i', $line, $reg ))
{
$rec = array();
$params = $reg[1];
@@ -78,7 +78,7 @@
}

// Add link to current node
- if (preg_match('/<DT.*><A HREF="([^"]+)"([^>]*)>([^<]+)<\/A>/i',$line, $reg ))
+ if (preg_match('/<DT.*><A HREF="([^"]+)"([^>]*)>([^<]*)<\/A>/i',$line, $reg ))
{
$rec = array();
$rec['url'] = $reg[1];
@@ -126,7 +126,7 @@
$comment = $this->toUTF8($reg[1]);

$line = array_shift($this->lines);
- while (count($this->lines) && !preg_match('/<\/?DL>/i', $line ))
+ while (count($this->lines) && !preg_match('/<\/?D[LT]>/i', $line ))
{
$comment .= "\r".$this->toUTF8($line);
$line = array_shift($this->lines);
Index: inc/page.inc.php
===================================================================
--- inc/page.inc.php (revision 412)
+++ inc/page.inc.php (working copy)
@@ -62,6 +62,16 @@
return $is?$_REQUEST[$name]:$default;
}

+function SB_reqValInt($name, $mandatory=false, $default='')
+{
+ $is = SB_reqChk($name);
+ if ($mandatory && !$is)
+ {
+ die('Expected field "'. $name .'" was not filled!');
+ }
+ return $is?intval($_REQUEST[$name]):$default;
+}
+
function SB_setVal($name, $value)
{
$_REQUEST[$name]=$value;
@@ -424,7 +434,15 @@
if ($trg === null)
{
$target = (SB_Page::isMSIE()||SB_Page::isOPERA()?'_main':'_content');
- if (isset($_REQUEST['target'])) $target = $_REQUEST['target'];
+ if (isset($_REQUEST['target']))
+ {
+ $newtarget = $_REQUEST['target'];
+
+ if (preg_match('/^\w+/', $newtarget))
+ {
+ $target = $newtarget;
+ }
+ }
$trg = $target;
}
return $trg;
Index: inc/tree.inc.php
===================================================================
--- inc/tree.inc.php (revision 412)
+++ inc/tree.inc.php (working copy)
@@ -651,9 +651,22 @@

if (SB_ALL_LINKS_FOR_ID != $uid)
{
+ // Ignore deleted roots (of other owners)
$where = array('^1'=>'uid <> '.$uid, '^2'=>'AND', 'deleted_by'=>null);

- // Ignore deleted roots (of other owners)
+ // We use cache, if it is not defined now, it will be defined next time
+ // !! PERFORMANCE
+ // If users are playing too much with ACL, this would be slow as well,
+ // because cache is always deleted.
+ $vis_nodes = $this->db->getCache('vis_nodes',$uid);
+ if (is_array($vis_nodes))
+ {
+ if (strlen($vis_nodes['cvalue'])>0)
+ {
+ $where['^3']='AND r.nid in ('.$vis_nodes['cvalue'].')';
+ }
+ }
+
$rset = $this->db->select( $select, $from, $where);

// Check all roots - can be slow with many users
@@ -987,8 +1000,8 @@
return $aclNodes;
}

- $rset = $this->db->select('distinct nid', 'sitebar_acl',
- array( '^1'=> 'gid in ('.implode(',',$gids).')'));
+ $rset = $this->db->select('distinct a.nid', 'sitebar_acl a join sitebar_node n',
+ array( '^1'=> 'gid in ('.implode(',',$gids).') and a.nid=n.nid and deleted_by IS NULL'));

$aclNodes = array();
while (($rec=$this->db->fetchRecord($rset)))
Index: inc/writers/blogroll.inc.php
===================================================================
--- inc/writers/blogroll.inc.php (revision 412)
+++ inc/writers/blogroll.inc.php (working copy)
@@ -48,7 +48,7 @@

function js($value)
{
- return "document.writeln('" . str_replace('\"','"',$value) . "');\r";
+ return "document.writeln('" . str_replace("'","\\'",$value) . "');\r";
}

function drawHead()
Index: inc/writers/opera.inc.php
===================================================================
--- inc/writers/opera.inc.php (revision 412)
+++ inc/writers/opera.inc.php (working copy)
@@ -46,7 +46,7 @@
echo "\tNAME=".$node->name."\r";
if ($node->comment)
{
- echo "\tDESCRIPTION=".$node->comment."\r";
+ echo "\tDESCRIPTION=".$this->quoteComment($node->comment)."\r";
}
echo "\r";
}
@@ -56,6 +56,14 @@
echo "-\r\r";
}

+ function quoteComment(&$comment)
+ {
+ $comment = str_replace("\r\n","\x2", $comment);
+ $comment = str_replace("\n","\x2", $comment);
+ $comment = str_replace("\r","\x2", $comment);
+ return $comment;
+ }
+
function drawLink(&$node, &$link)
{
echo "#URL\r";
@@ -63,7 +71,7 @@
echo "\tURL=".$link->url."\r";
if ($link->comment)
{
- echo "\tDESCRIPTION=".$link->comment."\r";
+ echo "\tDESCRIPTION=".$this->quoteComment($link->comment)."\r";
}
echo "\r";
}
Index: inc/writers/rss.inc.php
===================================================================
--- inc/writers/rss.inc.php (revision 412)
+++ inc/writers/rss.inc.php (working copy)
@@ -79,7 +79,7 @@
$this->drawTag('generator', null, 'SiteBar ' . SB_CURRENT_RELEASE . ' (Bookmark Server; http://sitebar.org/)');

// Time to live in minutes
- $this->drawTag('ttl', null, null, '60');
+ $this->drawTag('ttl', null, '60');
}

function drawLink(&$node, &$link)
@@ -96,29 +96,33 @@
// $this->drawTag('author', null, null);
$this->drawTag('link', null, $this->quoteText($link->url));

- $this->drawTag('description', null, $this->quoteText($link->comment));
+ if (strlen($link->comment))
+ {
+ $this->drawTag('description', null, $this->quoteText($link->comment));
+ }
+
$date = '';
$append = '';
switch ($this->tree->sortMode)
{
case 'changed':
- $date = $this->getDateISO8601($link->changed);
+ $date = $link->changed;
$append = '#' . $date;
break;
case 'tested':
- $date = $this->getDateISO8601($link->tested);
+ $date = $link->tested;
$append = '#' . $date;
break;
case 'hits':
$append = '#' . $link->hits;
- $date = $this->getDateISO8601($link->visited);
+ $date = $link->visited;
break;
case 'visited':
- $date = $this->getDateISO8601($link->visited);
+ $date = $link->visited;
$append = '#' . $date;
break;
case 'added':
- $date = $this->getDateISO8601($link->added);
+ $date = $link->added;
break;
default:
$date = ($link->added>$link->changed?$link->added:$link->changed);
Index: integrator.php
===================================================================
--- integrator.php (revision 412)
+++ integrator.php (working copy)
@@ -54,7 +54,10 @@

SB_Page::absBaseUrl($_COOKIE['sbi_url']);
SB_Skin::set($_COOKIE['sbi_skin']);
-SB_SetLanguage($_GET['lang']);
+if (preg_match('/^\w+/', $_GET['lang']))
+{
+ SB_SetLanguage($_GET['lang']);
+}

if (isset($_REQUEST['install']))
{
@@ -102,7 +105,7 @@
(
'label' =>'Mozilla Firefox',
'homepage' =>'http://www.mozilla.org/products/firefox/',
- 'platforms'=>'1.0,1.5/WinXP,Linux',
+ 'platforms'=>'1.0,1.5,2.0/WinXP,Linux',
'usage' => '',
'exclude' =>array(),
'extra' =>array('sitebar_client','bmsync','sitebar','sidebar','livebookmarks','m
ozlinker','search_engine'),
@@ -173,7 +176,7 @@
'sitebar_client' => array
(
'label' => 'SiteBar Client',
- 'url' => 'https://addons.mozilla.org/extensions/moreinfo.php?application=firefox&
amp;id=1545',
+ 'url' => 'https://addons.mozilla.org/firefox/3605/',
'desc' => SB_P('integrator::hint_sitebar'),
),

Index: news.php
===================================================================
--- news.php (revision 412)
+++ news.php (working copy)
@@ -24,6 +24,10 @@
SB_handleRootCookie('news.php');

$_REQUEST['w'] = 'news';
+
+require_once('./inc/writers/xbel.inc.php');
+require_once('./inc/writers/dir.inc.php');
+require_once('./inc/writers/news.inc.php');
include('index.php');

?>
Index: sql/downgrade_3.3.9.sql
===================================================================
--- sql/downgrade_3.3.9.sql (revision 0)
+++ sql/downgrade_3.3.9.sql (revision 0)
@@ -0,0 +1,2 @@
+UPDATE `sitebar_config`
+ SET `release` = '3.3.8';
Index: sql/install.sql
===================================================================
--- sql/install.sql (revision 412)
+++ sql/install.sql (working copy)
@@ -18,7 +18,7 @@
CREATE TABLE `sitebar_config` (
`gid_admins` int(10) unsigned NOT NULL DEFAULT '1',
`gid_everyone` int(10) unsigned NOT NULL DEFAULT '2',
- `release` varchar(10) NOT NULL DEFAULT '3.3.8',
+ `release` varchar(10) NOT NULL DEFAULT '3.3.9',
`changed` datetime NOT NULL default '0000-00-00 00:00:00',
`params` text
)
Index: sql/upgrade_3.3.8.sql
===================================================================
--- sql/upgrade_3.3.8.sql (revision 0)
+++ sql/upgrade_3.3.8.sql (revision 0)
@@ -0,0 +1,2 @@
+UPDATE `sitebar_config`
+ SET `release` = '3.3.9';
Index: translator.php
===================================================================
--- translator.php (revision 412)
+++ translator.php (working copy)
@@ -68,20 +68,20 @@
var $infofmt = './locale/%s/%s';
var $langs = array();
var $gid = null;
- var $dir = '.';
- var $dirCGI = '';
+ var $plugin = '';
+ var $pluginCGI = '';

function Translator()
{
- if (isset($_GET['dir']) || isset($_POST['dir']))
+ if (isset($_GET['plugin']) || isset($_POST['plugin']))
{
- $dir = isset($_GET['dir'])?$_GET['dir']:$_POST['dir'];
+ $plugin = isset($_GET['plugin'])?$_GET['plugin']:$_POST['plugin'];

- if ($dir != "." && $dir != "")
+ if ($plugin != "" && preg_match('/^\w+$/', $plugin))
{
- $this->dir = $dir;
+ $this->dir = './plugins/'.$plugin;
$this->fmt = $this->dir.'/locale/%s/%s';
- $this->dirCGI = "dir=".$this->dir."&";
+ $this->pluginCGI = "plugin=".$plugin."&";
}
}

@@ -283,8 +283,8 @@

<form method="get">
Translate
-<select name='dir' onChange="this.form.submit()">
-<option value='.'>SiteBar</option>
+<select name='plugin' onChange="this.form.submit()">
+<option value=''>SiteBar</option>
<?php

$dir = opendir('./plugins');
@@ -308,7 +308,7 @@
continue;
}

- echo "<option ". ($_GET['dir']==$plugdir?"selected":"") ." value='$plugdir'>Plugin $plugin</option>\n";
+ echo "<option ". ($_GET['plugin']==$plugin?"selected":"") ." value='$plugin'>Plugin $plugin</option>\n";
}
closedir($dir);
?>
@@ -443,9 +443,9 @@

if ($lang!=DEFAULT_LANGUAGE)
{
- ?>[<a href='translator.php?lang=<?php echo $lang?>&<?php echo $this->dirCGI ?>edit=<?php echo $part?>'>EDIT</a>]<?php
-if ($missing) : ?><br>[<a href='translator.php?lang=<?php echo $lang?>&<?php echo $this->dirCGI ?>cmd=add&edit=<?php echo $part?>'>ADD</a>]<?php endif;
-if ($update && !$this->parts[$part]['inline']) : ?><br>[<a href='translator.php?lang=<?php echo $lang?>&<?php echo $this->dirCGI ?>cmd=upd&edit=<?php echo $part?>'>UPD</a>]<?php endif;
+ ?>[<a href='translator.php?lang=<?php echo $lang?>&<?php echo $this->pluginCGI ?>edit=<?php echo $part?>'>EDIT</a>]<?php
+if ($missing) : ?><br>[<a href='translator.php?lang=<?php echo $lang?>&<?php echo $this->pluginCGI ?>cmd=add&edit=<?php echo $part?>'>ADD</a>]<?php endif;
+if ($update && !$this->parts[$part]['inline']) : ?><br>[<a href='translator.php?lang=<?php echo $lang?>&<?php echo $this->pluginCGI ?>cmd=upd&edit=<?php echo $part?>'>UPD</a>]<?php endif;
}
}
}
@@ -453,7 +453,7 @@
$server = defined("DOWNLOAD_SRV")?DOWNLOAD_SRV:"";

?>
- <td class='stat'>[<a href='<?php echo $server?>translator.php?<?php echo $this->dirCGI ?>download=<?php echo $lang?>'>Download</a>]</td>
+ <td class='stat'>[<a href='<?php echo $server?>translator.php?<?php echo $this->pluginCGI ?>download=<?php echo $lang?>'>Download</a>]</td>
</tr>
<?php
}
@@ -486,15 +486,22 @@
SB_Page::head('Edit Translation', 'locale');
?>
<h2>Edit Translation</h2>
-[<a href="translator.php?<?php echo $this->dirCGI ?>">Back to Translation List</a>]
+[<a href="translator.php?<?php echo $this->pluginCGI ?>">Back to Translation List</a>]
<p>
<?php
+ if (!isset($this->parts[$part]))
+ {
+ die("Unknown part in edit param!");
+ }
+
+ if (!preg_match('/^\w+$/',$lang))
+ {
+ die("Not allowed characters in lang param!");
+ }
+
$param = $this->parts[$part];
$file = sprintf($this->fmt,$lang,$param['file']);

- mkdir($this->dir.'/locale/'.$lang, 0777);
- chmod($this->dir.'/locale/'.$lang, 0777);
-
include($file);
eval('$data = $'.$part.';');
eval('$'.$part.'=array();');
@@ -572,9 +579,17 @@
else
{
$value = str_replace("\r\n","\n", $value);
- fwrite( $fh, "\$".$part."['".$label."'] = <<<_P\n");
+ fwrite( $fh, "\$".$part."['".$label."'] = <<<_SBHD\n");
+
+ // Do not allow here doc to be included in the string,
+ // otherwise any php code would be executed.
+ if (strstr($value,"_SBHD"))
+ {
+ die("Value must not contain _SBHD pattern!");
+ }
+
fwrite( $fh, $value);
- fwrite( $fh, "\n_P;\n\n");
+ fwrite( $fh, "\n_SBHD;\n\n");
}
}
}
@@ -601,6 +616,7 @@
<table class="edit">
<input type="hidden" name="dir" value="<?php $this->dir ?>">
<?php
+
$i = 0;

foreach ($default as $label => $value)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Nth Dimension Security Advisory (NDSA20071016)
Date: 16th October 2007
Author: Tim Brown <mailto:timb (at) nth-dimension.org (dot) uk [email concealed]>
URL: <http://www.nth-dimension.org.uk/> / <http://www.machine.org.uk/>
Product: SiteBar 3.3.8 <http://www.sitebar.org/>
Vendor: OndÅ?ej Brablc, David Szego and SiteBar Team <http://www.sitebar.org/>
Risk: High

Summary

This advisory comes in 4 related parts:

1) SiteBar application has single high risk issues with its translation
module. It can can be made to retrieve any file to which the web server user
has read access.

2) SiteBar application has multiple high risk issues with its translation
module. It can be made to execute arbitrary code to gain remote access
as the web server user typically nobody.

3) SiteBar application has multiple medium risk issues where it is vulnerable
to Javascript injection within the requested URL.

4) SiteBar application has single medium risk issue where it is vulnerable to
malicious redirects within the requested URL.

Technical Details

1) The SiteBar application translation module can be made to read any
arbitrary file that the web server user has read access to, as it makes
no sanity checks on the value passed within the dir parameter of the URL,
for example:

http://192.168.1.1/translator.php?dir=/etc/passwd%00

Note the use of %00 to terminate the malicious and so prevent the intended
string concatenation occuring.

2) The SiteBar application translation module can be forced into code
execution can occur in one of two ways. Firstly, it makes no sanity checks
on the value passed within the edit parameter prior to using the value as
part of an eval() call, for example:

http://192.168.1.1/translator.php?lang=zh_CN&cmd=upd&edit=$GET[%22lang%2
2];system(%22uname%20-a%22);

Secondly, whilst modifying strings within a translation, it makes no sanity
checks on the value passed for a given string to be embedded within a HERE
document within the languages strings library. It is therefore possible to
terminate the HERE document and pass arbitrary code which will be executed
whenever the languages strings library is included, for example:

POST http://192.168.1.1/translator.php?lang=test&edit=text HTTP/1.1
Host: 192.168.1.1
Referer: http://192.168.1.1/translator.php?lang=test&edit=text
Cookie: SB3COOKIE=1; SB3AUTH=3efab8d1dc9a149d7d1d7866a33d2539
Content-Type: application/x-www-form-urlencoded
Content-length: 47497

dir=&label%5B0%5D=The+Bookmark+Server+for+Personal+and+Team+Use&md5%5B0%
5D=823084516ae27478ec4c5fd40fb32ea8&value%5B0%5D=_P;

system('id');

?>

Note that _P terminates the HERE document.

3) The values of the URL requested are used in within the web pages returned
by the various scripts, in their unsanitised form. Specifically, it makes
no sanity checks on the value passed within the multiple parameters of the
URL, for example:

http://192.168.1.1/integrator.php?lang="><script>alert('xss')</script> - Allows '
http://192.168.1.1/command.php?command=New+Password&uid=&token="><script
>alert(document.cookie)</script> - Does not allow '
http://192.168.1.1/command.php?command=Folder%20Properties&nid_acl=%3Csc
ript%3Ealert(document.cookie)%3C/script%3E - Does not allow '
http://192.168.1.1/index.php?target=%22%3E%3Cscript%3Ealert(document.coo
kie)%3C/script%3E - Does not allow '
http://192.168.1.1/command.php?command='%3Cscript%3Ealert(document.cooki
e)%3C/script%3E - Does not allow ', this one turned out to be CVE-2006-3320.
http://192.168.1.1/command.php?command=Modify%20User&uid=%22%3E%3Cscript
%3Ealert('xss')%3C/script%3E - Allows '

Note that CVE-2006-3320 had not been resolved at the time of testing, in
September 2007, and so we included it in our vulnerability report to the vendor
for completeness.

4) Finally, the SiteBar can be made to redirect users to malicious locations,
as it makes no checks on the value passed within the forward parameter of the URL,
for example:

http://192.168.1.1/command.php?command=Log%20In&forward=http://www.googl
e.com/

Solutions

Following vendor notification on the 27th September 2007, the vendor promptly
responded with an initial patch on the 7th October which has been attached along
with this advisory and which resolved the reported issues. Nth Dimension would
recommend applying this patch as soon as possible. Alternatively, from 3.3.9
(available at http://sitebar.org/downloads.php) onwards also include this patch.
Nth Dimension would like to thank Ondraj from the SiteBar team for the way he
worked to resolve the issue.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFHFo3OVAlO5exu9x8RAhLWAJ0Vw4cessVBHnFMswYp6aDlmriDnwCfXpil
wyDF4P/iRQ5Ab7FqJFutWBA=
=Oqb/
-----END PGP SIGNATURE-----

[ reply ]


 

Privacy Statement
Copyright 2010, SecurityFocus