Hi Richard I am not sure about your patch. Setting a maximum length does not fix a potential xss issue. Why not using htmlspecialchars() to take care of escaping? I have attached a potential patch for that. Of course, it would be good to check the rest of the code as well and see whether it is prone to xss issues. Also, as far as I understand it, the CSRF issue is very constructed and doesn't offer an attack vendor without having admin rights already, correct? I have to admit that I don't understand that part of your patch there.
Cheers Steffen
--- ../old/ipplan-4.91a/admin/usermanager.php 2009-03-18 20:44:03.000000000 +0000 +++ ipplan-4.91a/admin/usermanager.php 2009-06-23 06:16:08.000000000 +0000 @@ -676,7 +676,9 @@ $formerror=""; $userid=trim($userid); + $userid=htmlspecialchars($userid); $userdescrip=trim($userdescrip); + $userdescrip=htmlspecialchars($userdescrip); $useremail=trim($useremail); $search=trim($search); if (AUTH_INTERNAL) { @@ -746,7 +748,9 @@ list($grp, $grpdescrip, $createcust, $grpview, $resaddr) = myRegister("S:grp S:grpdescrip S:createcust S:grpview I:resaddr"); $grp=trim($grp); + $grp=htmlspecialchars($grp); $grpdescrip=trim($grpdescrip); + $grpdescrip=htmlspecialchars($grpdescrip); $formerror=""; if (strlen($grp) < 2) {
signature.asc
Description: This is a digitally signed message part.