Package: ocsinventory-reports
Version: 2.0.5-1.1
Severity: important

Dear Maintainer,

ocsinventory-reports oversanitizes GET and POST data. In
require/header.php there are the following three lines 179-181:

//SECURITY
$protectedPost=strip_tags_array($_POST);
$protectedGet=strip_tags_array($_GET);

strip_tags_array is included in require/function_commun.php and applies
PHP's intrinsic strip_tags to an entire array. This means that
everything from a lesser sign to the next greater sign or the end of
the string (whichever comes first) will be removed from all data
supplied to the application by the user.

This means that for example passwords containing lesser signs will not
be handled correctly.

Steps to reproduce:

1. Setup a clean OCS inventory reports installation, make sure logging
   in with the initial password admin/admin works.
2. Use the following SQL command line to manually change the password
   to "<hello>":
   echo "UPDATE operators SET passwd=MD5('a<b12345') WHERE ID='admin';"\
      | mysql -p ocsweb
3. Try to login with the username admin and the password a<b12345.

Note that if you change the password in the interface (and not the
database directly), the same bug also affects the password change field.
Therefore, logging in still works, but two serious problems remain here:

a. If you upgrade from a previous version that did not do this (e.g.
   Squeeze), then the database will still contain MD5 hashes of older
   passwords that potentially contained this character. In that case,
   logging in is not possible.

b. Suppose an administrator wants to set a somewhat strong password
   such as "2<AB557Gdv!3fghnj" (made up just for this bug report), and
   they do that in the web interface. Then strip_tags on that password
   will remove everything after the lesser sign and the password that
   will actually be stored in the database is going to be just "2",
   which is incredibly weak. And they may not notice it, since typing
   in the full password in the login screen still works, because the
   login screen also truncates it.

Since this code appeared between 2.0.1 and 2.0.2, presumably this was
done to mitiagate CVE-2011-4024, a XSS vulnerability.

The problem here is that while using strip_tags does mitigate the XSS,
it is actually not the proper fix. See the OWASP for more details:
https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet

I have had a short look at the source code of ocsinventory-reports and
properly mitigating XSS without resorting to this quick&dirty strip_tags
mitigation seems to necessitate touching a large amount of the PHP
source code.

The main issue here is the password handling, there are not that many
places elsewhere in the software where user data is actually entered
in this way (most data is collected via ocsinventory-server, which is
a completely different codebase), so if you are interested, I can
provide a patch that just works around the password issue. That
wouldn't completely resolve this bug (since other data is also affected
by strip_tags here), but it would reduce its severity.

Thank you!


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to