Re: [aur-dev] [PATCH] Redirect back after login

2012-12-17 Thread canyonknight 
On Mon, Dec 17, 2012 at 1:10 PM, Marcel Korpel  wrote:
> On Sun, Dec 16, 2012 at 7:12 PM, canyonknight  wrote:
>> This implementation is susceptible to HTTP header injection.
>
> Ok. You mean in the current 'Location:' line without filtering 0x0a and 0x0d?
>

Response splitting shouldn't be an issue. PHP prevents multiple
headers from being sent at once in the header() function. I was
referring to the fact that it is an unsanitized $_GET variable being
used as a header. It can be manipulated and could redirect to a
website outside the AUR or some other clever attack.

That is one of the nice things about using a $_SESSION variable in
this case. The server could directly set the redirect location in a
$_SESSION variable without the user being able to tamper with it.

>> Also note
>> the usage of $_SERVER['REQUEST_URI'] had previously been eliminated
>> with commit 630f1cbae8473fb05e5f5af7244eccc60fe93812.
>
> If we can't trust $_SERVER['REQUEST_URI'], then how should we
> determine the current URL? Using $_SERVER['PATH_INFO'] and
> $_SERVER['QUERY_STRING']? Or are these also susceptible to
> manipulation?
>

Briefly, I always thought the following could be a decent solution:
- User is on a page and the route is saved as a $_SESSION variable
- User navigates to login page and logs in
- Login page uses the routing code to redirect to page saved in the
$_SESSION variable

I realize it isn't a GET parameter solution, but it is easy to do
securely. The only downside is if a user has multiple tabs open, it
will redirect to the last page opened. That and to implement properly
it would require a bit of work.

Regards,

Jason

Re: [aur-dev] [PATCH] Redirect back after login

2012-12-17 Thread Marcel Korpel 
On Sun, Dec 16, 2012 at 7:12 PM, canyonknight  wrote:
> This implementation is susceptible to HTTP header injection.

Ok. You mean in the current 'Location:' line without filtering 0x0a and 0x0d?

> Also note
> the usage of $_SERVER['REQUEST_URI'] had previously been eliminated
> with commit 630f1cbae8473fb05e5f5af7244eccc60fe93812.

If we can't trust $_SERVER['REQUEST_URI'], then how should we
determine the current URL? Using $_SERVER['PATH_INFO'] and
$_SERVER['QUERY_STRING']? Or are these also susceptible to
manipulation?

Regards, Marcel

Re: [aur-dev] [PATCH] Implemented typeahead suggest

2012-12-17 Thread Marcel Korpel 
On Sun, Dec 16, 2012 at 7:05 PM, canyonknight  wrote:
> So after thinking about this some more, what are your thoughts on
> adding a method to aurjson.class.php and having the JavaScript call
> rpc.php for the JSON data instead of a new suggest.php file?

Changing the function to a method in aurjson.class.php seems all right
to me, it's a logical place to put that function in. BTW, why is
aurjson.class.php the only OO PHP file in the AUR and the rest merely
procedural? Not that I'm in favor of one or the other (and my OOP
skills are limited to prototypal OOP in JavaScript).

However, it would imply quite a change of aurjson.class.php, as many
methods in it are specifically meant for the current RPC interface.
E.g., 'search' is not designed for this specific suggest search and
'json_results' produces an object rather than an array.

Or would you want to include a new public method called 'suggest_search'?

Regards, Marcel