[ postfixadmin-Bugs-1770514 ] edit-active.php vulnerable to URL injection
Brought to you by:
christian_boltz,
gingerdog
Menu
▾
▴
[ postfixadmin-Bugs-1770514 ] edit-active.php vulnerable to URL injection
|
From: SourceForge.net <no...@so...> - 2007-10-09 16:58:18
|
Bugs item #1770514, was opened at 2007-08-08 23:11 Message generated for change (Settings changed) made by gingerdog You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=937964&aid=1770514&group_id=191583 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None >Status: Closed >Resolution: Out of Date Priority: 5 Private: No Submitted By: Christian Boltz (christian_boltz) Assigned to: Nobody/Anonymous (nobody) Summary: edit-active.php vulnerable to URL injection Initial Comment: It's possible to inject an URL in edit-active.php where an user gets redirected to. Example: http://.../postfixadmin/admin/edit-active.php?alias=postmaster%40cboltz.de&domain=cboltz.de&return=http://www.google.de If the target page looks like Postfixadmin, the user might be tricked to enter privata data, mailbox passwords etc. The relevant code is quite new in SVN: diff -u -p -r1.1 edit-active.php --- admin/edit-active.php 16 Jan 2007 15:50:58 -0000 1.1 +++ admin/edit-active.php 8 Aug 2007 23:05:03 -0000 @@ -31,6 +31,7 @@ [...] + if (isset ($_GET['return'])) $fReturn = escape_string ($_GET['return']); [...] @@ -72,7 +73,14 @@ if ($error != 1) { - header ("Location: list-virtual.php?domain=$fDomain"); + if ( $fReturn != "" ) + { + header ("Location: $fReturn"); + } + else + { + header ("Location: list-virtual.php?domain=$fDomain"); + } exit; } Please validate $_GET['return'], using a list of allowed values. Sidenote: The "Location:" header requires a full URL to be specified. Giving only a filename can cause interesting[tm] effects in some browsers. ---------------------------------------------------------------------- Comment By: GingerDog (gingerdog) Date: 2007-10-08 20:39 Message: Logged In: YES user_id=1761957 Originator: NO It looks like it's fixed... if ($error != 1) { if ( preg_match( "/^list-virtual.php.*/", $fReturn ) || preg_match( "/^overview.php.*/", $fReturn ) || preg_match( "/^search.php.*/", $fReturn ) ) { //$fReturn appears OK, jump there header ("Location: $fReturn"); } else { if (authentication_has_role('global-admin')) { header ("Location: list-virtual.php?domain=$fDomain"); } else { header ("Location: overview.php?domain=$fDomain"); } } exit; ---------------------------------------------------------------------- Comment By: Christian Boltz (christian_boltz) Date: 2007-08-17 20:21 Message: Logged In: YES user_id=593261 Originator: YES No, I'm not aware of a browser that doesn't support relative redirects. But it is still off-spec and therefore might cause problems. But this is just a sidenote here - validating the return page is the important thing. Right now it looks like the ?return= parameter is only used from search.php. The best way to make it safe would be to use "?search=<search term>". ---------------------------------------------------------------------- Comment By: GingerDog (gingerdog) Date: 2007-08-17 15:41 Message: Logged In: YES user_id=1761957 Originator: NO My understanding of Location: was that the spec says browsers should expect a full url, however all browsers support relative Location headers. Do you know of some browsers that don't support relative redirects? ---------------------------------------------------------------------- Comment By: Christian Boltz (christian_boltz) Date: 2007-08-08 23:23 Message: Logged In: YES user_id=593261 Originator: YES *ARGH* - this affects both postfixadmin/edit-active.php and postfixadmin/admin/edit-active.php. Duplicated code means duplicated bugs... ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=937964&aid=1770514&group_id=191583 |
