From b42c89441fe1ed1da414e0fb6569826f57717a3a Mon Sep 17 00:00:00 2001 From: Rudis Muiznieks Date: Mon, 20 Jul 2020 22:13:03 -0500 Subject: [PATCH] replaced shell quote with catfile, added more tests --- Dockerfile | 2 +- server.pl | 14 ++++---- test/run.sh | 98 +++++++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 100 insertions(+), 14 deletions(-) diff --git a/Dockerfile b/Dockerfile index d4cd2a1..a39ffb4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ from alpine:latest run apk add wget gnupg sqlite unzip build-base perl perl-dev perl-app-cpanminus -run cpanm --notest IPC::Run DBD::SQLite Net::DNS::Resolver Crypt::Eksblowfish::Bcrypt JSON URI::Escape HTML::Entities String::ShellQuote Net::Server HTTP::Server::Simple Crypt::Random +run cpanm --notest IPC::Run DBD::SQLite Net::DNS::Resolver Crypt::Eksblowfish::Bcrypt JSON URI::Escape HTML::Entities Net::Server HTTP::Server::Simple Crypt::Random run mkdir -p /opt/data/plans copy schema.sql /opt/data diff --git a/server.pl b/server.pl index b9021d3..8debf68 100644 --- a/server.pl +++ b/server.pl @@ -14,8 +14,8 @@ my $pid_file = $ENV{'PID_FILE'} || './data/dotplan.pid'; my $log_file = $ENV{'LOG_FILE'} || './data/dotplan.log'; my $database = $ENV{'DATABASE'} || './data/users.db'; my $plan_dir = $ENV{'PLAN_DIR'} || './data/plans'; -my $sendmail = $ENV{'SENDMAIL'} || '/usr/bin/true'; -my @sendmail_args = split(/,/, $ENV{'SENDMAIL_ARGS'}); +my $sendmail = $ENV{'SENDMAIL'} || '/bin/true'; +my @sendmail_args = defined $ENV{'SENDMAIL_ARGS'} ? split(/,/, $ENV{'SENDMAIL_ARGS'}) : (); my $pw_token_expiration_minutes = $ENV{'PW_TOKEN_EXPIRATION_MINUTES'} || 10; my $auth_token_default_expiration_minutes = $ENV{'AUTH_TOKEN_DEFAULT_EXPIRATION_MINUTES'} || 5; @@ -64,7 +64,7 @@ if (defined $ENV{'LOCAL_DOMAINS'}) { use JSON qw(encode_json decode_json); use URI::Escape qw(uri_escape); use HTML::Entities qw(encode_entities); - use String::ShellQuote qw(shell_quote); + use File::Spec::Functions qw(catfile); ############### # Common Errors @@ -400,7 +400,7 @@ EOF print_json_response($cgi, 400, {error => "Pubkey exceeds maximum length of $maximum_pubkey_length."}); } else { my (undef, $keyfile) = tempfile('tmpXXXXXX', SUFFIX => '.gpg', TMPDIR => 1, OPEN => 0); - my $basename = "$plan_dir/" . shell_quote($email); + my $basename = catfile($plan_dir, $email); IPC::Run::run ['gpg2', '--dearmor'], \$pubkey, '>', $keyfile, '2>>', '/dev/null' or die "gpg2 exited with $?"; if(IPC::Run::run ['gpg2', '--no-default-keyring', '--keyring', "$keyfile", '--verify', "$basename.asc", "$basename.plan"], '>', '/dev/null', '2>>', '/dev/null') { $plan->{'verified'} = 1; @@ -456,7 +456,7 @@ EOF my @arg = ($sendmail); push @arg, @sendmail_args; push @arg, $recipient; - IPC::Run::run \@arg, \$email, '>>', '/dev/null', '2>>', '/dev/null' or die "sendmail exited with $?"; + IPC::Run::run \@arg, \$email or die "sendmail exited with $?"; } # get mime type for response from querystring and accept header @@ -553,7 +553,7 @@ EOF my $_plancache = {}; sub util_save_plan { my ($email, $plan, $signature) = @_; - my $basename = "$plan_dir/" . shell_quote($email); + my $basename = catfile($plan_dir, $email); if (defined $plan) { open(my $plan_file, '>', "$basename.plan") or die $!; @@ -581,7 +581,7 @@ EOF sub util_read_plan { my $email = shift; if (!defined $_plancache->{$email}) { - my $basename = "$plan_dir/" . shell_quote($email); + my $basename = catfile($plan_dir, $email); if (-f "$basename.plan") { my $details = {}; diff --git a/test/run.sh b/test/run.sh index d045ee1..c0d5dbd 100755 --- a/test/run.sh +++ b/test/run.sh @@ -55,7 +55,7 @@ assert_equal() { actual=$1;shift expected=$1;shift if [ "$actual" != "$expected" ]; then - printf "${RED}✗ CHECK${NC} ${BOLD}$check_name${NC}\n\n\"${YELLOW}$actual${NC}\" != \"${YELLOW}$expected${NC}\"\n\n" + printf "${RED}✗ CHECK${NC} ${BOLD}$check_name${NC}\n\n\"${YELLOW}"; echo -n "$actual"; printf "${NC}\" != \"${YELLOW}"; echo -n "$expected"; printf "${NC}\"\n\n" ((++FAILED)) return 1 fi @@ -68,7 +68,7 @@ assert_equal_jq() { expected=$1;shift actual=$(echo "$TEST_CONTENT" | jq -r "$selector") if [ "$actual" != "$expected" ]; then - printf "${RED}✗ CHECK${NC} ${BOLD}$selector${NC}\n\n\"${YELLOW}$actual${NC}\" != \"${YELLOW}$expected${NC}\"\n\n" + printf "${RED}✗ CHECK${NC} ${BOLD}$selector${NC}\n\n\"${YELLOW}"; echo -n "$actual"; printf "${NC}\" != \"${YELLOW}"; echo -n "$expected"; printf "${NC}\"\n\n" ((++FAILED)) return 1 fi @@ -81,7 +81,7 @@ assert_notequal_jq() { expected=$1;shift actual=$(echo "$TEST_CONTENT" | jq -r "$selector") if [ "$actual" == "$expected" ]; then - printf "${RED}✗ CHECK${NC} ${BOLD}$selector${NC} is null\n" + printf "${RED}✗ CHECK${NC} ${BOLD}$selector${NC}\n\n\"${YELLOW}"; echo -n "$selector"; printf "${NC}\" = \"${YELLOW}"; echo -n "$expected"; printf "${NC}\"\n\n" ((++FAILED)) return 1 fi @@ -89,6 +89,32 @@ assert_notequal_jq() { return 0; } +assert_exists() { + check_name=$1;shift + dir=$1;shift + file=$1;shift + if [ ! -e "$BASEDIR/$dir/$file" ]; then + printf "${RED}✗ CHECK${NC} ${BOLD}$check_name${NC}\n\n\"${YELLOW}"; echo -n "$BASEDIR/$dir/$file"; printf "${NC}\" does not exist\n\n" + ((++FAILED)) + return 1 + fi + printf "${GREEN}✓ CHECK${NC} ${BOLD}$check_name${NC}\n" + return 0; +} + +assert_not_exists() { + check_name=$1;shift + dir=$1;shift + file=$1;shift + if [ -e "$BASEDIR/$dir/$file" ]; then + printf "${RED}✗ CHECK${NC} ${BOLD}$check_name${NC}\n\n\"${YELLOW}"; echo -n "$BASEDIR/$dir/$file"; printf "${NC}\" exists\n\n" + ((++FAILED)) + return 1 + fi + printf "${GREEN}✓ CHECK${NC} ${BOLD}$check_name${NC}\n" + return 0; +} + ############ # Test Setup ############ @@ -107,7 +133,6 @@ if [ -z "$USE_DOCKER" ]; then LOG_FILE="$BASEDIR/data/test.log" \ DATABASE="$BASEDIR/data/test.db" \ PLAN_DIR="$BASEDIR/data/plans" \ - SENDMAIL=/usr/bin/true \ perl "$BASEDIR/../server.pl" -d >>/dev/null else docker build -t dotplan-online-test "$BASEDIR/.." @@ -118,7 +143,6 @@ else -e LOG_FILE="/opt/data/test.log" \ -e DATABASE="/opt/data/test.db" \ -e PLAN_DIR="/opt/data/plans" \ - -e SENDMAIL=/usr/bin/true \ dotplan-online-test fi @@ -248,6 +272,63 @@ curl_test 'Verify signed plan' 200 'application/json' -XPOST -d "$post_data" loc && assert_equal_jq '.plan' 'this is a plan that is signed' +BADGUY='badguy@example.com%2F..%2Fgotya' +curl_test 'Avoid directory traversal 1 account creation' 404 'application/json' -XPOST -d '{"password":"test1234"}' localhost:$PORT/users/$BADGUY + +BADGUY='badguy@example.com%252F..%252Fgotya' +BADGUY_ESC='badguy@example.com%2F..%2Fgotya' +curl_test 'Avoid directory traversal 2 account creation' 200 'application/json' -XPOST -d '{"password":"test1234"}' localhost:$PORT/users/$BADGUY \ + && assert_notequal_jq '.email' 'null' + +pw_token=$(echo "select pw_token from users where email='$BADGUY_ESC'" | sqlite3 "$BASEDIR/data/test.db") + +curl_test 'Verify directory traversal address 2' 200 'application/json' -XPUT -d "{\"token\":\"$pw_token\"}" localhost:$PORT/users/$BADGUY \ + && assert_equal_jq '.success' 1 + +curl_test 'Get directory traversal 2 authentication token' 200 'application/json' -u "$BADGUY_ESC:test1234" localhost:$PORT/token + +token=$(echo "$TEST_CONTENT" | jq -r '.token') + +curl_test 'Create directory traversal 2 plan' 200 'application/json' -XPUT -d "{\"plan\":\"something\",\"auth\":\"$token\"}" localhost:$PORT/plan/$BADGUY \ + && assert_equal_jq '.success' 1 \ + && assert_not_exists 'malicious file' 'data' 'gotya' \ + && assert_exists 'benign plan file' 'data/plans' "$BADGUY_ESC.plan" + +BADGUY="badguy@example.com\\..\\gotya" +curl_test 'Avoid directory traversal 3 account creation' 200 'application/json' -XPOST -d '{"password":"test1234"}' localhost:$PORT/users/$BADGUY + +pw_token=$(echo "select pw_token from users where email='$BADGUY'" | sqlite3 "$BASEDIR/data/test.db") + +curl_test 'Verify directory traversal address 3' 200 'application/json' -XPUT -d "{\"token\":\"$pw_token\"}" localhost:$PORT/users/$BADGUY \ + && assert_equal_jq '.success' 1 + +curl_test 'Get directory traversal 3 authentication token' 200 'application/json' -u "$BADGUY:test1234" localhost:$PORT/token + +token=$(echo "$TEST_CONTENT" | jq -r '.token') + +curl_test 'Create directory traversal 3 plan' 200 'application/json' -XPUT -d "{\"plan\":\"something\",\"auth\":\"$token\"}" localhost:$PORT/plan/$BADGUY \ + && assert_equal_jq '.success' 1 \ + && assert_not_exists 'malicious file' 'data' 'gotya' \ + && assert_exists 'benign plan file' 'data/plans' "$BADGUY.plan" + +BADGUY="badguy%40example.com%27%3Bdrop%20table%20users%3B" +BADGUY_ESC="badguy@example.com';drop table users;" +curl_test 'Avoid SQL injection account creation' 200 'application/json' -XPOST -d '{"password":"test1234"}' "localhost:$PORT/users/$BADGUY" \ + && assert_equal_jq '.email' "$BADGUY_ESC" + +pw_token=$(echo "select pw_token from users where email='${BADGUY_ESC//\'/\'\'}'" | sqlite3 "$BASEDIR/data/test.db") + +curl_test 'Verify SQL injection address' 200 'application/json' -XPUT -d "{\"token\":\"$pw_token\"}" localhost:$PORT/users/$BADGUY \ + && assert_equal_jq '.success' 1 + +curl_test 'Get SQL injection authentication token' 200 'application/json' -u "$BADGUY_ESC:test1234" localhost:$PORT/token + +token=$(echo "$TEST_CONTENT" | jq -r '.token') + +curl_test 'Create SQL injection plan' 200 'application/json' -XPUT -d "{\"plan\":\"something\",\"auth\":\"$token\"}" localhost:$PORT/plan/$BADGUY \ + && assert_equal_jq '.success' 1 \ + && assert_exists 'benign plan file' 'data/plans' "$BADGUY_ESC.plan" + ############### # Test Teardown ############### @@ -264,5 +345,10 @@ else docker kill dotplan_online_test fi -printf "Tests complete.\n" +if [ $FAILED -gt 0 ]; then + printf "${RED}" +else + printf "${GREEN}" +fi +printf "Tests complete. $FAILED failed.${NC}\n" exit $FAILED