Bug#530271: xss patch
On Wed, 24 Jun 2009 07:46:01 am Richard Ellerbrock wrote: The existing patch is correct - using htmlspecialchars will have the effect of placing escaped stings in the database. It will also have the effect of double escaping each time you edit a field. My patch replaces the display template method block() which does not escape with the text() method which uses htmlspecialchars internally. See /ipplan/layout/class.layout You are right, thanks for pointing this out. As for the length check. This was a potential, unrelated database overflow I discovered during investigation of the xss issue - totally unrelated. Could you elaborate on this? Could this cause any issues security wise? Cheers Steffen signature.asc Description: This is a digitally signed message part.
Bug#530271: xss patch
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.0 +
+++ ipplan-4.91a/admin/usermanager.php 2009-06-23 06:16:08.0 +
@@ -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.
Bug#530271: xss patch
The existing patch is correct - using htmlspecialchars will have the effect of placing escaped stings in the database. It will also have the effect of double escaping each time you edit a field. My patch replaces the display template method block() which does not escape with the text() method which uses htmlspecialchars internally. See /ipplan/layout/class.layout As for the length check. This was a potential, unrelated database overflow I discovered during investigation of the xss issue - totally unrelated. As for the CSRF issue. Its so specific, too hard to fix (I might be wrong here), requires admin rights with which you could delete a user anyway and will potentially never get used in an application that has such a focus and small user base. So this issue is not fixed. I have checked the rest of IPplan and am fairly convinced that there are no other block method issues. I will check again. Note that the usermanager component was written by another developer (not me), thus the potential for these types of issues. 2009/6/23 Steffen Joeris steffen.joe...@skolelinux.de: 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 -- Richard Ellerbrock -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
