[aur-dev] [PATCH v2] Fake pkgbase actions for unconfirmed users

[Gordian Edenhofer]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <https://lists.archlinux.org/pipermail/aur-dev/attachments/20150626/4c617b78/attachment.asc>