|
From: David G. <da...@co...> - 2007-10-04 06:05:02
|
Christian Boltz wrote : > Hello, >=20 > just going through the svn commits... =20 I'm glad someone else looks at them :) > > --- trunk/templates/admin_edit-admin.tpl 2007-09-28 20:35:19 UTC (rev > > 105) +++ trunk/templates/admin_edit-admin.tpl 2007-09-28 20:35:43 UTC > > (rev 106) @@ -10,7 +10,7 @@ > > </tr> > > <tr> > > <td><?php print $PALANG['pAdminEdit_admin_password'] . ":"; > > ?></td>=20 > > - <td><input class=3D"flat" type=3D"password" name=3D"fPassword"= =20 > > - /></td> =20 > > + <td><input class=3D"flat" type=3D"password" name=3D"fPassword"= =20 > > + value=3D"<?=3D $fPassword; ?>"/></td>=20 >=20 > I consider it security-critical to include the password in the HTML code= =20 > (browser cache etc.). Luckily, this code seems to be buggy - at least,=20 > it never included the password for me. Ah, it should say $tPassword. I'd never intentionally display the unencrypted password in the form. <insert comment about postfix admin's horrible templating and sanitisation structure /> <Smarty + Prepared statements would make me very happy /> > Argh, it seems admin/edit-admin.php needs some fixes... >=20 > I just fixed some password-related bugs in edit-admin.php: > - When entering a password in the first field and leaving the second one= =20 > empty, the password was changed anyways.=20 Er.. I thought my logic was something along the lines of : 1) Get password from template - if it matches the encrypted password belonging to that admin assume it's not changed and do nothing. 2) If it doesn't match the encrypted password, see if password2 is available, and if they match, update the user's password to the new value (after pacrypt'ing it). > - It was also changed to an empty password if you left both fields=20 > empty. This is a bad idea because you often modify some admin settings= =20 > without even knowing his password. =20 There would normally have been the 1st password field with a value within it, so this shouldn't happen (unless someone wants it to have an empty password) I guess my web browser was a little too eager in caching/filling password boxes in for me. > One bug is remaining in admin/edit-admin.php: > When editing an admin, it does not take the "active" status from the=20 > database. This means editing an admin always disables it (unless you=20 > correct the checkbox yourself). > Can you please fix this? Hmm.. I think this is a pgsql vs mysql-ism - in that it works for PgSQL. I'll try and review the code again today. Thank you! David. --=20 David Goodwin=20 [ david at codepoets dot co dot uk ] [ http://www.codepoets.co.uk ] |
