[aur-dev] [PATCH 3/4] fix case where user does not exist
elij
elij.mx at gmail.com
Wed May 11 14:54:34 EDT 2011
On Wed, May 11, 2011 at 7:22 AM, Lukas Fleischer
<archlinux at cryptocrack.de> wrote:
> On Tue, May 10, 2011 at 09:01:29PM -0700, elij wrote:
>> the query was being performed when $id was not set, resulting in an
>> invalid sql query being performed.
>> ---
>> web/lib/acctfuncs.inc | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/web/lib/acctfuncs.inc b/web/lib/acctfuncs.inc
>> index 5bcff8b..b2f0548 100644
>> --- a/web/lib/acctfuncs.inc
>> +++ b/web/lib/acctfuncs.inc
>> @@ -786,6 +786,9 @@ function valid_passwd( $userID, $passwd )
>> */
>> function user_suspended( $id )
>> {
>> + if (!$id) {
>> + return false;
>> + }
>> $dbh = db_connect();
>> $q = "SELECT Suspended FROM Users WHERE ID = " . $id;
>> $result = db_query($q, $dbh);
>
> Looks ok, but I'd rather say we should locate the code path that led to
> the unset parameter and add some additional validation there to avoid
> further unexpected behaviour.
The source is in try_login (also in lib/acctfuncs.inc):
if ( isset($_REQUEST['user']) || isset($_REQUEST['passwd']) ) {
$userID = valid_user($_REQUEST['user']);
if ( user_suspended( $userID ) ) {
$login_error = "Account Suspended.";
}
valid_user (also in the same file) can return (no value) in some cases.
that large if/elseif block in try_login could probably have some more
conditions added to test for existence of $userID before calling
user_suspended on it. Still.. it might make sense to have a guard (the
test added with this patch) in the function itself in case usage of it
down the road changes (another code path or something).
More information about the aur-dev
mailing list