Skip to content

Commit

Permalink
Fix a number of open redirect vulnerabilities.
Browse files Browse the repository at this point in the history
  • Loading branch information
yunosh committed Jul 3, 2017
1 parent d7206b1 commit 6dc88b8
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 21 deletions.
10 changes: 3 additions & 7 deletions add.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,13 @@
$vars->set('source', key($addSources));
}
$source = $vars->get('source');
$url = $vars->get('url');
$url = ($url = Horde::verifySignedUrl($vars->get('url')))
? Horde::url($url, true)
: Horde::url('index.php', true);

/* Exit with an error message if there are no sources to add to. */
if (!$addSources) {
$notification->push(_("There are no writeable address books. None of the available address books are configured to allow you to add new entries to them. If you believe this is an error, please contact your system administrator."), 'horde.error');
$url = $url
? Horde::url($url, true)
: Horde::url('index.php', true);
$url->redirect();
}

Expand All @@ -42,9 +41,6 @@

if (!is_null($driver)) {
if (Turba::hasMaxContacts($driver)) {
$url = $url
? Horde::url($url, true)
: Horde::url('index.php', true);
$url->redirect();
}

Expand Down
8 changes: 5 additions & 3 deletions delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@
try {
$driver->delete($vars->key);
$notification->push(sprintf(_("Deleted contact: %s"), $object->getValue('name')), 'horde.success');
$url = strlen($vars->url)
? new Horde_Url($vars->url)
: Horde::url($prefs->getValue('initial_page'), true);
if ($url = Horde::verifySignedUrl($vars->url)) {
$url = new Horde_Url($url);
} else {
$url = Horde::url($prefs->getValue('initial_page'), true);
}
$url->redirect();
} catch (Turba_Exception $e) {
$notification->push(sprintf(_("There was an error deleting this contact: %s"), $e->getMessage()), 'horde.error');
Expand Down
9 changes: 6 additions & 3 deletions edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
$source = $vars->source;
$key = $vars->key;
$groupedit = ($vars->actionID == 'groupedit');
$url = new Horde_Url($vars->get('url', Horde::url($prefs->getValue('initial_page'), true)));
$url = ($url = Horde::verifySignedUrl($vars->get('url')))
? new Horde_Url($url)
: Horde::url($prefs->getValue('initial_page'), true);

/* Edit the first of a list of contacts? */
if ($groupedit && (!$key || $key == '**search')) {
Expand Down Expand Up @@ -76,13 +78,14 @@
/* Execute() checks validation first. */
try {
$edited = $form->execute();
$url = isset($vars->url)
$url = ($url = Horde::verifySignedUrl($vars->url))
? new Horde_Url($url, true)
: $contact->url('Contact', true);
$url->add('section', $form->getOpenSection())
->unique()
->redirect();
} catch (Turba_Exception $e) {}
} catch (Turba_Exception $e) {
}

$title = sprintf($contact->isGroup() ? _("Edit Contact List \"%s\"") : _("Edit \"%s\""), $contact->getValue('name'));
Horde::startBuffer();
Expand Down
9 changes: 6 additions & 3 deletions lib/Form/AddContact.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,12 @@ public function execute()
try {
$ob = $driver->getObject($key);
$notification->push(sprintf(_("%s added."), $ob->getValue('name')), 'horde.success');
$url = empty($info['url'])
? $ob->url('Contact', true)
: new Horde_Url($info['url']);
if (!empty($info['url']) &&
$url = Horde::verifySignedUrl($info['url'])) {
$url = new Horde_Url($url);
} else {
$url = $ob->url('Contact', true);
}
$url->redirect();
} catch (Horde_Exception_NotFound $e) {
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Form/EditContactGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public function execute()
'source' => $info['source'],
'original_source' => $info['original_source'],
'objectkeys' => $info['objectkeys'],
'url' => $info['url'],
'url' => Horde::signUrl($info['url']),
'actionID' => 'groupedit'
));

Expand Down
4 changes: 3 additions & 1 deletion merge.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
$mergeInto = Horde_Util::getFormData('merge_into');
$driver = $injector->getInstance('Turba_Factory_Driver')->create($source);

if ($url = Horde_Util::getFormData('url')) {
if ($url = Horde::verifySignedUrl(Horde_Util::getFormData('url'))) {
$url = new Horde_Url($url, true);
$url = $url->unique();
} else {
$url = Horde::url('', true);
}

try {
Expand Down
3 changes: 2 additions & 1 deletion package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@
<package>
<name>Horde_Core</name>
<channel>pear.horde.org</channel>
<min>2.12.0</min>
<min>2.30.0</min>
<max>3.0.0alpha1</max>
<exclude>3.0.0alpha1</exclude>
</package>
Expand Down Expand Up @@ -2198,6 +2198,7 @@
<date>2017-03-20</date>
<license uri="http://www.horde.org/licenses/apache">ASL</license>
<notes>
* [jan] SECURITY: Fix open redirects.
* [jan] Fix creating address books with the external API.
</notes>
</release>
Expand Down
2 changes: 1 addition & 1 deletion templates/browse/row.inc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ if ($ob->hasValue('__key')) {
$edit_url = Horde::link(Horde::url('edit.php')->add(array(
'key' => $ob->getValue('__key'),
'source' => $ob->getSource(),
'url' => Horde::selfUrl(true, false, true)
'url' => Horde::signUrl(Horde::selfUrl(true, false, true))
), _("Edit"))) . '<span class="iconImg editImg"></span></a>';
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion templates/search/duplicate/contact_header.html.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<form action="<?php echo $this->mergeUrl ?>">
<input type="hidden" name="source" value="<?php echo $this->source ?>" />
<input type="hidden" name="key" value="<?php echo $this->h($this->id) ?>" />
<input type="hidden" name="url" value="<?php echo Horde::selfUrl(true) ?>" />
<input type="hidden" name="url" value="<?php echo Horde::signUrl(Horde::selfUrl(true)) ?>" />
<input type="hidden" name="merge_into" value="<?php echo $this->h($this->mergeTarget) ?>" />
<input type="submit" class="horde-default" value="<?php echo _("<< Merge this into the first contact") ?>" />
</form>
Expand Down

0 comments on commit 6dc88b8

Please sign in to comment.