On Wed, Jul 13, 2005 at 06:28:21PM +0200, Martin Schulze wrote: > I guess we're facing a severe problem here. > > Even though you say that my fixes were not sufficient, you have > ***removed*** a fair amount of the patches I've applied after > reading the code that uses unsanitised variables. I now see > that you've placed sanitising into the config file entirely, > would have been nice to note this.
i guess i didn't in the email updating this, but did so in sanitize.php
itself:
/*
* backported security-related changes from cacti
* by sean finney <[EMAIL PROTECTED]>
*
* to preserve my own sanity, all sanity checks are done in here, which
* is included by the main configuration, which is included by everything.
* variables that don't exist will not raise failures, so only in the case
* that the input exists and is not what it is supposed to be will there
* be an error.
*/
> Additionally you seem to be using get_request_var only which
> uses the $_GET array, but not the $_REQUEST array, and hence
> can be bypassed by POST or cookie input if I am not mistaken.
> This was not the case in the version I sent you.
the problem with using _REQUEST is that someone could provide a valid
_POST variable, but sneak the malcious content into _GET, which would
then pass a _REQUEST test (assuming order gpc), but if the system uses
_GET it still uses the malicious content. this is most of the cause of
the second set of advisorires.
however, now that i think about it, since i think most variables in
the woody version of cacti are using register_globals, a variable like
$id will be set in the same order as $_REQUEST, so maybe that isn't a
bad idea.
now that i think about it even more, what would be best is to run the
sanity check on all of the _GET, _POST, _COOKIE variables, and fail
if any of them have bad values. that would make the patch even
simpler.
> In addition to that you also clutter sanitize.php with sanitising
> variables that aren't even used. That's not ok.
aren't even used on a specific page or aren't used at all in cacti? in
the case of the former, it causes no problems (apart from a couple extra
cycles, which i think is OK in the interest of a cleaner patch). in the
case of the latter, the lines should be removed--though again it doesn't
hurt to have it there.
> PS: ... and the distribution needs to be set to oldstable-security
okay. so this is what i will do in the next week:
- modify sanitize.php to check all three _FOO arrays for bad values and
quit out if any of them are bad.
- double check sanitize.php for globally unused variables.
- update the distribution name.
how does that sound?
sean
--
signature.asc
Description: Digital signature
