|
|
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 |
|