[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#652464: ITP: aguilas -- A web-based LDAP user management system



After i correct all this issues, should i fill in another ITP?

On 17/12/11 18:24, Roberto C. Sánchez wrote:
> On Sat, Dec 17, 2011 at 06:08:35PM -0430, Luis Alejandro Martínez Faneyth wrote:
>> On 17/12/11 16:19, Sune Vuorela wrote:
>>>
>>>     $sel_q = "SELECT * FROM NewUser"
>>>                       . " WHERE mail='" . $mail . "'"
>>>                       . " AND uid='" . $uid . "'"
>>>                       . " AND token='" . $token . "'"
>>>                       . " ORDER BY token DESC LIMIT 0,1";
>>
>> Thanks for having a look :)
>>
>> Well, i perform a very strict validation before that query is made.
>> Lines 20 - 54:
>>
>> http://code.google.com/p/aguilas/source/browse/NewUserDo.php#20
>> http://code.google.com/p/aguilas/source/browse/NewUserDo.php#54
>>
>> You are still scared?
>>
> I would be.  Bind variables exist for a reason.  Aside from that, your
> validation of email addresses is wrong:
> 
> // Invalid e-mail
> } elseif (preg_match("/\w+([-+.]\w+)*@\w+([-.]\w+)*\.\w+([-.]\w+)*/", $mail) == 0) {
> 
> First off, there is nothing in the RFC that says that the email address
> must start with a letter, which your regex requires.  In addition to
> that, you do not allow other allowed special characters:
> 
>  !#$%&'*/=?^_`{|}~"(),:;<>@[\]
> 
> You also don't properly check for consecutive dots, so I could pass the
> email a...b@foo.com and it pass your check, and still be wrong.
> 
> What you have done is reinvent the wheel, and badly at that.
> 
> If it were up to me, I would reject this package based on that one line
> of code alone.
>>
>> CODE IS POETRY
>>
> I find it terribly ironic that you have that satement in your email
> signature.
> 
> Regards,
> 
> -Roberto
> 

-- 
Luis Alejandro Martínez Faneyth
Blog: http://www.huntingbears.com.ve/
Twitter/Identi.ca: @LuisAlejandro
ED51 8FE7 4107 715D 0464  8366 F614 5A95 E78D AA2E


CODE IS POETRY

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: