Sean Finney wrote:
> On Tue, Jul 19, 2005 at 07:54:31AM +0200, Martin Schulze wrote:
> > Ok, I'll wait.
>
> so, a 6 hour plane flight later, i've learned 3 things:
>
> 1 - there are a number of other variables that also need to be included.
> 2 - there are a number of calls where variables are indirectly passed
> to mysql_foo functions via other functions (which causes a problem
> for the current sanity checking method)
> 3 - there is another, ridiculously obvious security vulnerability in
> the woody version.
Thanks a lot for your investigation!
> 1 is easy to fix, we can just add on the extra variables to the file.
> of the 900 or so calls to mysql_foo functions, i had about 170 left
> to look at when my battery crapped out.
>
> 2 is trickier. we could either repeat the process i'm about finished
> with wrt mysql_foo for all the functions that pass variables to
> mysql_foo, or we could do the sanity checking in the function. as
> the former sounds ugly and even more time consuming i'm going to
> side with thte latter.
The less work and the less intrusive the patch the better.
> what i think i'm going to do is split sanitize.php into sanitize and
> sanitize-functions. sanitize will include_once sanitize-functions,
> so then sanitize can be included multiple times (otherwise i believe
> that php will bitch about functions being redefined), and i'll just
> slip in a line in each mysql-calling function to include sanitize,
> and add the variables in said functions to sanitize.php.
Sounds good.
> as for 3, well... there's a variable, which is stored in a cookie.
> the cookie name is cactilogin, and the value is an integer. want to
> guess what it does? a fix for this shouldn't be too hard, this kind
> of info should be stored in the session and not in the cookie.
Hmm, having the user id stored in a cookie is common practice.
The variable obviously needs to be sanitised as well.
> anyway, i'll have a fair amount of free time tomorrow, but will need
> a little sleep first :)
Ok. For reference I'm attaching the interdiff between the woody
version and the current updated version on security.debian.org (in
the private queue).
Regards,
Joey
--
Whenever you meet yourself you're in a time loop or in front of a mirror.
Please always Cc to me when replying to me on the lists.
diff -u cacti-0.6.7/include/config.php cacti-0.6.7/include/config.php
--- cacti-0.6.7/include/config.php
+++ cacti-0.6.7/include/config.php
@@ -23,6 +23,21 @@
*/?>
<?
+/* whether or not we pull from a db, we need re-initilize */
+global $config;
+
+/* test for suspicious user-supplied variables that would otherwise
+ affect program execution, and if so zero out config for safety */
+if(isset($_GET["do_not_read_config"]) || isset($_POST["do_not_read_config"])
+ || isset($_GET["config"]) || isset($_POST["config"])){
+ $config = array();
+}
+$colors = array();
+
+## debian security backport ##
+require_once("sanitize.php");
+## debian security backport ##
+
## Debian packaging ##
include("/etc/cacti/config.php");
## Debian packaging ##
@@ -30,9 +45,6 @@
/* make sure this variable reflects your operating system type: 'unix' or
'win32' */
$cacti_server_os = "unix";
-/* whether or not we pull from a db, we need re-initilize */
-global $config;
-
if ($do_not_read_config != true) {
if (isset($config) == false) {
/* make a connection to the database */
diff -u cacti-0.6.7/debian/changelog cacti-0.6.7/debian/changelog
--- cacti-0.6.7/debian/changelog
+++ cacti-0.6.7/debian/changelog
@@ -1,3 +1,25 @@
+cacti (0.6.7-2.4) oldstable-security; urgency=high
+
+ * Non-maintainer upload by the Security Team
+ * Switched to using $_REQUEST instead of $_GET and $_POST since this
+ version only uses $foo which is similar to $_REQUEST[foo]
+ * Reduced the number of tested variables to those actually used.
+
+ -- Martin Schulze <[EMAIL PROTECTED]> Fri, 15 Jul 2005 16:06:58 +0200
+
+cacti (0.6.7-2.3) oldstable-security; urgency=high
+
+ * update prepared for the security team by new maintainer.
+ * include backported updates against the two latest cacti security
+ releases. this includes the following CAN id's:
+ - CAN-2005-1524 (idefense remote file inclusion)
+ - CAN-2005-1525 (idefense SQL injection)
+ - CAN-2005-1526 (idefense remote code execution)
+ - CAN-2005-2148 (hardened-php advisories 032005 and 042005)
+ - CAN-2005-2149 (hardened-php advisory 052005)
+
+ -- sean finney <[EMAIL PROTECTED]> Mon, 11 Jul 2005 20:35:12 -0400
+
cacti (0.6.7-2.2) stable-security; urgency=medium
* Non-maintainer upload by Stable Release Manager
only in patch2:
unchanged:
--- cacti-0.6.7.orig/include/sanitize.php
+++ cacti-0.6.7/include/sanitize.php
@@ -0,0 +1,123 @@
+<?php
+
+/*
+ * 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.
+ */
+
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004 Ian Berry |
+ | |
+ | This program is free software; you can redistribute it and/or |
+ | modify it under the terms of the GNU General Public License |
+ | as published by the Free Software Foundation; either version 2 |
+ | of the License, or (at your option) any later version. |
+ | |
+ | This program is distributed in the hope that it will be useful, |
+ | but WITHOUT ANY WARRANTY; without even the implied warranty of |
+ | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
+ | GNU General Public License for more details. |
+ +-------------------------------------------------------------------------+
+ | cacti: a php-based graphing solution |
+ +-------------------------------------------------------------------------+
+ | Most of this code has been designed, written and is maintained by |
+ | Ian Berry. See about.php for specific developer credit. Any questions |
+ | or comments regarding this code should be directed to: |
+ | - [EMAIL PROTECTED] |
+ +-------------------------------------------------------------------------+
+ | - raXnet - http://www.raxnet.net/ |
+ +-------------------------------------------------------------------------+
+*/
+
+/* get_request_var_request - returns the current value of a PHP $_POST
variable, optionally
+ returning a default value if the request variable does not exist
+ @arg $name - the name of the request variable. this should be a valid key
in the
+ $_REQUEST array
+ @arg $default - the value to return if the specified name does not exist in
the
+ $_REQUEST array
+ @returns - the value of the request variable */
+function get_request_var_request($name, $default = "")
+{
+ if (isset($_REQUEST[$name]))
+ {
+ return $_REQUEST[$name];
+ } else
+ {
+ return $default;
+ }
+}
+
+function input_validate_input_equals($value, $c_value) {
+ if ($value != $c_value) {
+ die_html_input_error();
+ }
+}
+
+function input_validate_input_number($value) {
+ if ((!is_numeric($value)) && ($value != "")) {
+ die_html_input_error();
+ }
+}
+
+function input_validate_input_regex($value, $regex) {
+ if ((!ereg($regex, $value)) && ($value != "")) {
+ die_html_input_error();
+ }
+}
+
+function die_html_input_error() {
+ global $config;
+
+ ?>
+ <table width="98%" align="center">
+ <tr>
+ <td>
+ Validation error.
+ </td>
+ </tr>
+ </table>
+ <?php
+
+ include_once("./include/bottom_footer.php");
+ exit;
+}
+
+input_validate_input_number(get_request_var_request("branch_id"));
+input_validate_input_number(get_request_var_request("graph_height"));
+input_validate_input_number(get_request_var_request("graph_start"));
+input_validate_input_number(get_request_var_request("graph_template_id"));
+input_validate_input_number(get_request_var_request("graph_width"));
+input_validate_input_number(get_request_var_request("graphid"));
+input_validate_input_number(get_request_var_request("hide"));
+input_validate_input_number(get_request_var_request("id"));
+input_validate_input_number(get_request_var_request("rra_id"));
+input_validate_input_number(get_request_var_request("tree_id"));
+input_validate_input_number(get_request_var_request("user_id"));
+
+if(isset($graph_template_id)){
+ input_validate_input_number($graph_template_id);
+}
+if(isset($matches[1])){
+ input_validate_input_number($matches[1]);
+}
+if(isset($matches[3])){
+ input_validate_input_number($matches[3]);
+}
+if(@is_array($_REQUEST["rra_id"])){
+ foreach($_REQUEST["rra_id"] as $debsec_key => $debsec_val){
+ input_validate_input_number($_REQUEST["rra_id"][$debsec_key]);
+ }
+}
+
+// The following alows more than the above test
+// input_validate_input_regex(get_request_var_request("rra_id"),
"^([0-9]+|all)$");
+input_validate_input_regex(get_request_var_request("type"), "^(in|out)$");
+
+?>
