From e0686062b453f236d3a2ca9b32147db5cf61937a Mon Sep 17 00:00:00 2001 From: James E Keenan Date: Thu, 7 Jul 2016 20:47:31 -0400 Subject: [PATCH] Dancer2::Core::Error::_censor(): Improve detection of sensitive data. In addition to checking the keys of each hash at each level, we should also check the keys of hashes blessed into classes. We should also check for the values of sensitive keys in key-value pairs in query clauses, e.g., '&pass=my_top_hush_pwrd'. t/error.t: Add subtest block explicitly testing _censor(). --- lib/Dancer2/Core/Error.pm | 14 ++- t/error.t | 258 +++++++++++++++++++++++++++++++++++++- 2 files changed, 268 insertions(+), 4 deletions(-) diff --git a/lib/Dancer2/Core/Error.pm b/lib/Dancer2/Core/Error.pm index e4ac31db0..1162ccd9d 100644 --- a/lib/Dancer2/Core/Error.pm +++ b/lib/Dancer2/Core/Error.pm @@ -9,6 +9,7 @@ use Data::Dumper; use Dancer2::FileUtils qw/path open_file/; use Sub::Quote; use Module::Runtime 'require_module'; +use Scalar::Util 'reftype'; has app => ( is => 'ro', @@ -425,17 +426,24 @@ sub _censor { my $censored = 0; for my $key ( keys %$hash ) { - if ( ref $hash->{$key} eq 'HASH' ) { + if ( + (ref $hash->{$key} eq 'HASH') + or + ((reftype($hash->{$key}) || '') eq 'HASH') + ) { # Take a copy of the data, so we can hide sensitive-looking stuff: $hash->{$key} = { %{ $hash->{$key} } }; $censored += _censor( $hash->{$key} ); } - elsif ( $key =~ /(pass|card?num|pan|secret)/i ) { + elsif ( + ($key =~ /(pass|card?num|pan|secret)/i ) + or + ( defined($hash->{$key}) and $hash->{$key} =~ /(pass|card?num|pan|secret)=/i ) + ) { $hash->{$key} = "Hidden (looks potentially sensitive)"; $censored++; } } - return $censored; } diff --git a/t/error.t b/t/error.t index 5c61b507a..a637b79ca 100644 --- a/t/error.t +++ b/t/error.t @@ -10,6 +10,7 @@ use Dancer2::Core::Request; use Dancer2::Core::Error; use JSON (); # Error serialization +use Capture::Tiny 'capture_stderr'; my $env = { 'psgi.url_scheme' => 'http', @@ -123,7 +124,7 @@ subtest 'Throwing an error with a response' => sub { exception => 'our exception', show_errors => 1 )->throw($resp) }; - + isa_ok($err, 'Dancer2::Core::Response', "Error->throw() accepts a response"); }; @@ -181,6 +182,261 @@ subtest 'Error with exception object' => sub { like $err->content, qr/a test exception object/, 'Error content contains exception message'; }; +subtest 'Screen out sensitive data' => sub { + + my $settings; + { + no warnings 'once'; + $settings = { + appdir => '/home/dancer2/mywebapp/', + apphandler => 'PSGI', + appname => 'mywebapp', + behind_proxy => 0, + cardnum => 1234567890123456, + charset => 'utf-8', + content_type => 'text/html', + environment => 'development', + host => '0.0.0.0', + layout => 'main', + log => 'core', + logger => 'console', + no_server_tokens => 0, + pan => 'foobar', + plugins => { + Database => { + charset => 'utf-8', + connection_check_threshold => 10, + connections => { + dancer => { + database => 'dancer_study', + driver => 'Pg' + } + }, + dbi_params => { + AutoCommit => 1, + RaiseError => 1, + pg_enable_utf8 => 1 + }, + handle_class => undef, + host => 'localhost', + log_queries => 1, + on_connect_do => [], + password => 'my_top_hush_pwrd', + port => 5432, + username => 'exotic_tango_dancer' + } + }, + port => '3000', + public_dir => '/home/dancer2/mywebapp/public', + route_handlers => [ + [ + 'AutoPage', + 1 + ] + ], + secret => 'Of Their Eyes', + session => bless( { + config => {}, + cookie_name => 'dancer.session', + cookie_path => '/', + hooks => { + 'engine.session.after_create' => [], + 'engine.session.after_destroy' => [], + 'engine.session.after_flush' => [], + 'engine.session.after_retrieve' => [], + 'engine.session.before_create' => [], + 'engine.session.before_destroy' => [], + 'engine.session.before_flush' => [], + 'engine.session.before_retrieve' => [] + }, + is_http_only => 1, + is_secure => 0, + log_cb => sub { "DUMMY" }, + request => bless( { + _body_params => { + pass => 'my_top_hush_pwrd', + path => '/hello', + user => 'exotic_tango_dancer' + }, + _chunk_size => 4096, + _http_body => bless( { + body => undef, + buffer => '', + chunk_buffer => '', + chunked => '', + cleanup => 1, + content_length => '62', + content_type => 'application/x-www-form-urlencoded', + length => 62, + param => { + pass => 'my_top_hush_pwrd', + path => '/hello', + user => 'exotic_tango_dancer' + }, + param_order => [ + 'user', + 'pass', + 'path' + ], + part_data => {}, + state => 'done', + tmpdir => '/tmp', + upload => {} + }, 'HTTP::Body::UrlEncoded' ), + _params => { + pass => 'my_top_hush_pwrd', + path => '/hello', + user => 'exotic_tango_dancer' + }, + _read_position => 62, + _route_params => {}, + body => 'user=exotic_tango_dancer&pass=my_top_hush_pwrd&path=%2Fhello', + cookies => { + 'dancer.session' => bless( { + http_only => 0, + name => 'dancer.session', + path => '/', + secure => 0, + value => [ + 'V37iHwAAI2gT8ottTrBIEqBdFNXzYndI' + ] + }, 'Dancer2::Core::Cookie' ) + }, + env => { + CONTENT_LENGTH => '62', + CONTENT_TYPE => 'application/x-www-form-urlencoded', + HTTP_ACCEPT => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8', + HTTP_ACCEPT_ENCODING => 'gzip, deflate', + HTTP_ACCEPT_LANGUAGE => 'en-US,en;q=0.5', + HTTP_CONNECTION => 'keep-alive', + HTTP_COOKIE => 'dancer.session=V37iHwAAI2gT8ottTrBIEqBdFNXzYndI', + HTTP_DNT => '1', + HTTP_HOST => 'localhost:5000', + HTTP_REFERER => 'http://localhost:5000/hello', + HTTP_USER_AGENT => 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0', + PATH_INFO => '/login', + QUERY_STRING => '', + REMOTE_ADDR => '127.0.0.1', + REMOTE_PORT => 50315, + REQUEST_METHOD => 'POST', + REQUEST_URI => '/login', + SCRIPT_NAME => '', + SERVER_NAME => 0, + SERVER_PORT => 5000, + SERVER_PROTOCOL => 'HTTP/1.1', + 'plack.request.body' => bless( { + pass => 'my_top_hush_pwrd', + path => '/hello', + user => 'exotic_tango_dancer' + }, 'Hash::MultiValue' ), + 'plack.request.http.body' => bless( { + body => undef, + buffer => '', + chunk_buffer => '', + chunked => '', + cleanup => 1, + content_length => '62', + content_type => 'application/x-www-form-urlencoded', + length => 62, + param => { + pass => 'my_top_hush_pwrd', + path => '/hello', + user => 'exotic_tango_dancer' + }, + param_order => [ + 'user', + 'pass', + 'path' + ], + part_data => {}, + state => 'done', + tmpdir => '/tmp', + upload => {} + }, 'HTTP::Body::UrlEncoded' ), + 'plack.request.upload' => bless( {}, 'Hash::MultiValue' ), + 'psgi.errors' => *::STDERR, + # 'psgi.input' => bless( \*{'Stream::Buffered::PerlIO::$io'}, 'FileHandle' ), + 'psgi.input' => '', + 'psgi.multiprocess' => '', + 'psgi.multithread' => '', + 'psgi.nonblocking' => '', + 'psgi.run_once' => '', + 'psgi.streaming' => 1, + 'psgi.url_scheme' => 'http', + 'psgi.version' => [ + 1, + 1 + ], + 'psgix.harakiri' => 1, + 'psgix.input.buffered' => 1, + 'psgix.io' => bless( \*Symbol::GEN4, 'IO::Socket::INET' ) + }, + headers => bless( { + accept => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8', + 'accept-encoding' => 'gzip, deflate', + 'accept-language' => 'en-US,en;q=0.5', + connection => 'keep-alive', + 'content-length' => '62', + 'content-type' => 'application/x-www-form-urlencoded', + cookie => 'dancer.session=V37iHwAAI2gT8ottTrBIEqBdFNXzYndI', + dnt => '1', + host => 'localhost:5000', + referer => 'http://localhost:5000/hello', + 'user-agent' => 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0' + }, 'HTTP::Headers::Fast' ), + id => 3, + is_behind_proxy => '', + route_parameters => bless( {}, 'Hash::MultiValue' ), + uploads => {}, + vars => {} + }, 'Dancer2::Core::Request' ), + session => bless( { + data => {}, + id => 'V37iHwAAI2gT8ottTrBIEqBdFNXzYndI', + is_dirty => 0 + }, 'Dancer2::Core::Session' ) + }, 'Dancer2::Session::Simple' ), + show_errors => 1, + startup_info => 1, + static_handler => 1, + template => 'simple', + traces => 0, + views => '/home/dancer2/mywebapp/views', + warnings => 1, + }; + } + + my $censored_count = Dancer2::Core::Error::_censor($settings); + # In $settings there are 7 instances of 'password' or 'pass' as key in + # hash or in query string. There is 1 instance each of 'cardnum', 'pan' + # or 'secret' as keys. + is $censored_count, 10, "Got expected censored count"; + my $sensitive = qr/Hidden \(looks potentially sensitive\)/; + like $settings->{cardnum}, qr/$sensitive/, "'cardnum' hidden"; + like $settings->{pan}, qr/$sensitive/, "'pan' hidden"; + like $settings->{secret}, qr/$sensitive/, "'secret' hidden"; + like $settings->{plugins}{Database}{password}, qr/$sensitive/, "'password' hidden"; + like $settings->{session}{request}{_body_params}{pass}, qr/$sensitive/, "'pass' hidden"; + like $settings->{session}{request}{_http_body}{param}{pass}, qr/$sensitive/, "'pass' hidden"; + like $settings->{session}{request}{_params}{pass}, qr/$sensitive/, "'pass' hidden"; + like $settings->{session}{request}{body}, qr/$sensitive/, "query string with 'pass' hidden"; + like $settings->{session}{request}{env}{'plack.request.body'}{pass}, qr/$sensitive/, "'pass' hidden"; + like $settings->{session}{request}{env}{'plack.request.http.body'}{param}{pass}, qr/$sensitive/, "'pass' hidden"; + + my ($arg, $rv, $stderr); + my $exp_error = qr/_censor given incorrect input/; + $arg = ''; + $stderr = capture_stderr { $rv = Dancer2::Core::Error::_censor($arg); }; + ok ! defined $rv, "Dancer2::Core::Error::_censor() returned undef due to false argument"; + like $stderr, qr/$exp_error/, "Got expected error message"; + + $arg = []; + $stderr = capture_stderr { $rv = Dancer2::Core::Error::_censor($arg); }; + ok ! defined $rv, "Dancer2::Core::Error::_censor() returned undef due to non-hashref argument"; + like $stderr, qr/$exp_error/, "Got expected error message"; + +}; + done_testing;