Sean Finney wrote:
> i guess i didn't in the email updating this, but did so in sanitize.php
> itself:
Yes, I saw that later. I hope, my tone wasn't too harsh.
> > 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.
Yes, but the woody version does not use $_GET *anywhere* except in
the alleged sanitising code you included. It uses $foo instead of
$_GET["foo"] all the time, which means for me - if I'm not mistaken -
that we should use either $foo or $_REQUEST["foo"] in the sanitising
code.
> 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.
True.
> 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.
It seems to me that running them on $_REQUEST only is sufficient. Or
do you know of a possibility that $foo can include something which is
not in $_REQUEST when inserted via GET/POST/cookie/$whatever?
> > 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
Aren't used at all.
See this for example:
finlandia!joey(pts/15):/src/debian/security/work/cacti/cacti-0.6.7> find -type
f|xargs grep cdef_id
./include/sanitize.php:input_validate_input_number(get_request_var("cdef_id"));
finlandia!joey(pts/15):/src/debian/security/work/cacti/cacti-0.6.7>
The only use of $cdef_id is in the sanitising code. For such cases we
don't need sanitising.
> 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
I already accepted that the correction due to its size will be done
centralised and hence not on each page.
> 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.
I'd go for _REQUEST only.
> - double check sanitize.php for globally unused variables.
> - update the distribution name.
>
> how does that sound?
Good.
However, as I don't like the "next week" part too much, I'll try to
work on the update on my own and send you the diff for comments.
Should reduce the time you need to spend on the issue as well.
Regards,
Joey
--
Computers are not intelligent. They only think they are.
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
