On Fri, 2015-06-26 at 23:08 +0200, Lukas Fleischer wrote:
On Fri, 26 Jun 2015 at 22:14:20, Gordian Edenhofer wrote:
On Fri, 2015-06-26 at 21:26 +0200, Lukas Fleischer wrote:
Cool, this is kind of what I expected! I wonder whether we can directly set a referer here, though? Shouldn't something like
get_uri('/login/', true) . '?referer=' . urlencode($uri)
work?
I can not make sense out of the "?referer" parameter, since my fix of FS#32481 uses $_SERVER['HTTP_REFERER'] and does not rely on an additional parameter to a GET request.
It uses $_SERVER['HTTP_REFERER'] unless $_REQUEST['referer'] is set. So we should be able to overwrite the referer by setting the HTTP GET parameter.
I forgot that I used $_REQUEST, I though that it was $_POST. My bad! Though if I think of it, it just might be a good idea to switch to $_POST since then $_GET parameters like "?refer" would not be concidered and only $_SERVER['HTTP_REFERER'] or a POST "referer" would be accepted. Shell I submit another patch for that or is the gain in security negligible?
Having said that, I now see a potential problem with the GET parameter approach of implementing FS#32481. You could build a malicious login link that redirects to a certain page and send that link to a privileged user (i.e. a TU or a Developer). I am not aware of any action that cannot be undone and doesn't require any additional confirmation, so that probably isn't very critical. We should fix it anyway...
Since most of the relevant package actions are perform through POST req uests (which are redirected as GET requests), I could not think of any potential security issues. Furthermore the referer is partially checked for outgoing links:
if (strpos($referer, aur_location()) !== 0) { $referer = '/'; }
Yeah, however, it is already kind of annoying if you can make a TU flag a package out-of-date without noticing. And we might introduce something more critical that does not require a POST request in the future. Better safe than sorry. This is something to fix in a separate patch set, though.
Flagging, voting, notifying and adopting a package is all done through POST requests AFAIK. Deleting or merging a package is not even available for unauthenticated users. Hence a malicious URL would not flag a package since the corresponding variable is not set.