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) {

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to