41f5ccc
From 42ce18f24d3eae8be33526a198bf21e4f2330230 Mon Sep 17 00:00:00 2001
41f5ccc
From: Steve Schnepp <steve.schnepp@pwkf.org>
41f5ccc
Date: Sat, 25 Feb 2017 11:20:52 +0100
41f5ccc
Subject: [PATCH] Fix wrong parameter expansion in CGI
41f5ccc
MIME-Version: 1.0
41f5ccc
Content-Type: text/plain; charset=UTF-8
41f5ccc
Content-Transfer-Encoding: 8bit
41f5ccc
41f5ccc
As Tomaž Šolc <tomaz.solc@tablix.org> said :
41f5ccc
41f5ccc
	Munin package in Jessie has a local file write vulnerability when CGI graphs are
41f5ccc
	enabled. Setting multiple "upper_limit" GET parameters allows overwriting any
41f5ccc
	file accessible to the www-data user.
41f5ccc
41f5ccc
And sstj <stevie.trujillo@gmail.com> said :
41f5ccc
41f5ccc
	Running munin-2.0.25 on Gentoo. I observed this message in the logs
41f5ccc
41f5ccc
	2016/07/26 21:57:54 [PERL WARNING] CGI::param called in list context
41f5ccc
	from /usr/libexec/munin/cgi/munin-cgi-graph line 450, this can lead to
41f5ccc
	vulnerabilities. See the warning in "Fetching the value or values of a
41f5ccc
	single named parameter" at /usr/lib64/perl5/vendor_perl/5.20.2/CGI.pm
41f5ccc
	line 404.
41f5ccc
41f5ccc
	This allows injecting options into munin-cgi-graph (similar to
41f5ccc
	http://munin-monitoring.org/ticket/1238 ), by doing something like
41f5ccc
	this:
41f5ccc
41f5ccc
	&upper_limit=500&upper_limit=--output-file&upper_limit=/tmp/test.txt
41f5ccc
41f5ccc
	which wrote the graph to /tmp/test.txt
41f5ccc
41f5ccc
Closes: #721, D:855705, CVE-2017-6188
41f5ccc
---
41f5ccc
 master/_bin/munin-cgi-graph.in | 22 ++++++++++++++--------
41f5ccc
 1 file changed, 14 insertions(+), 8 deletions(-)
41f5ccc
41f5ccc
diff --git a/master/_bin/munin-cgi-graph.in b/master/_bin/munin-cgi-graph.in
41f5ccc
index 092458b..09bc3f2 100755
41f5ccc
--- a/master/_bin/munin-cgi-graph.in
41f5ccc
+++ b/master/_bin/munin-cgi-graph.in
41f5ccc
@@ -335,14 +335,20 @@ sub draw_graph {
41f5ccc
 		   '--output-file', $filename );
41f5ccc
 
41f5ccc
     # Sets the correct size on a by_graph basis
41f5ccc
-    push @params, "--size_x", CGI::param("size_x")
41f5ccc
-      if (defined(CGI::param("size_x")));
41f5ccc
-    push @params, "--size_y", CGI::param("size_y")
41f5ccc
-      if (defined(CGI::param("size_y")));
41f5ccc
-    push @params, "--upper_limit", CGI::param("upper_limit")
41f5ccc
-      if (CGI::param("upper_limit"));
41f5ccc
-    push @params, "--lower_limit", CGI::param("lower_limit")
41f5ccc
-      if (CGI::param("lower_limit"));
41f5ccc
+
41f5ccc
+    # using a temporary variable to avoid expansion in list context and fix CVE-2017-6188
41f5ccc
+    my $size_x = CGI::param("size_x");
41f5ccc
+    push @params, "--size_x", $size_x if defined $size_x;
41f5ccc
+
41f5ccc
+    my $size_y = CGI::param("size_y");
41f5ccc
+    push @params, "--size_y", $size_y if defined $size_y;
41f5ccc
+
41f5ccc
+    my $upper_limit = CGI::param("upper_limit");
41f5ccc
+    push @params, "--upper_limit", $upper_limit if defined $upper_limit;
41f5ccc
+
41f5ccc
+    my $lower_limit = CGI::param("lower_limit");
41f5ccc
+    push @params, "--lower_limit", $lower_limit if defined $lower_limit;
41f5ccc
+
41f5ccc
 
41f5ccc
     graph_main(\@params);
41f5ccc