Re: Bug#652464: ITP: aguilas -- A web-based LDAP user management system
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
--
Roberto C. Sánchez
http://people.connexer.com/~roberto
http://www.connexer.com
Reply to: