|
From: David G. <da...@co...> - 2007-10-05 06:20:26
|
>=20 > > > I consider it security-critical to include the password in the HTML > > > code (browser cache etc.). Luckily, this code seems to be buggy - > > > at least, it never included the password for me. > > > > Ah, it should say $tPassword. I'd never intentionally display the > > unencrypted password in the form. >=20 > This isn't really better - if someone uses unencrypted passwords, there= =20 > will be a cleartext password again. Ah, bugg-rit. I'd forgotten about that 'feature' As these are the passwords for the admin user, how about we change is so admin passwords are _always_ encrypted with something decent?=20 As admin passwords aren't used for mailboxes, it wouldn't have any impact outside of postfixadmin. >=20 > The correct and secure solution is not to insert the password at all. >=20 My grievance with this is that when ever an admin is edited, I think your previous patch, implied that a users password had to be changed/set. Which is horrible. > > <Smarty + Prepared statements would make me very happy /> >=20 > I don't know Smarty good enough to add a statement, but I'm afraid it=20 > would add some overhead. There are other ways to make the templates=20 > easier... =20 To which I'd say : 1) Postfixadmin isn't going to be in a high demand, zillions of users style situation, so performance doesn't matter (that much!) 2) Using Smarty would stop the automatic spread of variables from the=20 controller to the view - which would make it far more obvious what is given to the view layer, and what isn't. 3) We can tell Smarty to sanitise all data it displays by default, rather than us having to run htmlentities everywhere, so we can stop XSS/HTML injection like problems very easily. > Prepared statements would be a good idea - we would get rid of SQL=20 > injections easily. Indeed. Question is, what version(s) of PHP/MySQL to we need to support. If we're thinking of older ones, then we'd have to use something like PEAR::DB or PEAR::MDB2 - which emulate prepared-statement-ness. I added a min_password_length config setting in.... which should help too ($CONF['min_password_length']) David. --=20 David Goodwin=20 [ david at codepoets dot co dot uk ] [ http://www.codepoets.co.uk ] |
