diff --git a/Makefile.am b/Makefile.am index fc78825..738f301 100644 --- a/Makefile.am +++ b/Makefile.am @@ -66,6 +66,7 @@ handwritten_tests = \ t/dnsexit2.pl \ t/get_ip_from_if.pl \ t/geturl_connectivity.pl \ + t/geturl_response.pl \ t/group_hosts_by.pl \ t/interval_expired.pl \ t/is-and-extract-ipv4.pl \ diff --git a/configure.ac b/configure.ac index 649b7e3..5ca6da7 100644 --- a/configure.ac +++ b/configure.ac @@ -36,7 +36,18 @@ AC_PROG_MKDIR_P AC_PATH_PROG([FIND], [find]) AS_IF([test -z "${FIND}"], [AC_MSG_ERROR(['find' utility not found])]) -AC_PATH_PROG([CURL], [curl]) +AC_ARG_WITH([curl], + [AS_HELP_STRING([[--with-curl[=CURL]]], [use CURL as absolute path to curl executable])], + [], + [with_curl=yes]) +AS_CASE([${with_curl}], + [[yes]], [AC_PATH_PROG([CURL], [curl])], + [[no]], [CURL=], + [ + AC_MSG_CHECKING([for curl]) + CURL=${with_curl} + AC_MSG_RESULT([${CURL}]) + ]); AS_IF([test -z "${CURL}"], [AC_MSG_ERROR([curl not found])]) AX_WITH_PROG([PERL], perl) diff --git a/ddclient.in b/ddclient.in index 53115d0..fa6e5ec 100755 --- a/ddclient.in +++ b/ddclient.in @@ -125,6 +125,7 @@ if ($program =~ /test/i) { $cachedir = '.'; $savedir = 'URL'; } +our @curl = (subst_var('@CURL@', 'curl')); our $emailbody = ''; my $last_emailbody = ''; @@ -2635,7 +2636,7 @@ sub curl_cmd { my @params = @_; my $tmpfile; my $tfh; - my $system_curl = quotemeta(subst_var('@CURL@', 'curl')); + my $curl = join(' ', @curl); my %curl_codes = ( ## Subset of error codes from https://curl.haxx.se/docs/manpage.html 2 => "Failed to initialize. (Most likely a bug in ddclient, please open issue at https://github.com/ddclient/ddclient)", 3 => "URL malformed. The syntax was not correct", @@ -2653,11 +2654,11 @@ sub curl_cmd { 67 => "The user name, password, or similar was not accepted and curl failed to log in.", 77 => "Problem with reading the SSL CA cert (path? access rights?).", 78 => "The resource referenced in the URL does not exist.", - 127 => "$system_curl was not found", + 127 => "$curl was not found", ); - debug("CURL: %s", $system_curl); - fatal("curl not found") if ($system_curl eq ''); + debug("CURL: %s", $curl); + fatal("curl not found") if ($curl[0] eq ''); return '' if (scalar(@params) == 0); ## no parameters provided # Hard code to /tmp rather than use system TMPDIR to protect from malicious @@ -2673,9 +2674,20 @@ sub curl_cmd { print($tfh @params); } close($tfh); - my $reply = qx{ $system_curl --config $tmpfile 2>/dev/null; }; + # Use open's list form (as opposed to qx, backticks, or the scalar form of open) to avoid the + # shell and reduce the risk of a shell injection vulnerability. ':raw' mode is used because + # HTTP is defined in terms of octets (bytes), not characters. In raw mode, each byte from curl + # is mapped to a same-valued codepoint (byte value 0x78 becomes character U+0078, 0xff becomes + # U+00ff). The caller is responsible for decoding the byte sequence if necessary. + open(my $cfh, '-|:raw', @curl, '--config', $tmpfile) + or fatal("failed to run curl ($curl): $!"); + # According to , adding ':raw' to the open + # mode is buggy with Perl < v5.14. Call binmode on the filehandle just in case. + binmode($cfh) or fatal("binmode failed: $!"); + my $reply = do { local $/; <$cfh>; }; + close($cfh); # Closing $cfh waits for the process to exit and sets $?. if ((my $rc = $?>>8) != 0) { - warning("CURL error (%d) %s", $rc, $curl_codes{$rc} // "Unknown return code. Check $system_curl is installed and its manpage."); + warning("CURL error (%d) %s", $rc, $curl_codes{$rc} // "Unknown return code. Check $curl is installed and its manpage."); } return $reply; } diff --git a/t/geturl_response.pl b/t/geturl_response.pl new file mode 100644 index 0000000..beb1a92 --- /dev/null +++ b/t/geturl_response.pl @@ -0,0 +1,27 @@ +use Test::More; +SKIP: { eval { require Test::Warnings; } or skip($@, 1); } +eval { require 'ddclient'; } or BAIL_OUT($@); + +# Fake curl. Use the printf utility, which can process escapes. This allows Perl to drive the fake +# curl with plain ASCII and get arbitrary bytes back, avoiding problems caused by any encoding that +# might be done by Perl (e.g., "use open ':encoding(UTF-8)';"). +my @fakecurl = ('sh', '-c', 'printf %b "$1"', '--'); + +my @test_cases = ( + { + desc => 'binary body', + # Body is UTF-8 encoded ✨ (U+2728 Sparkles) followed by a 0xff byte (invalid UTF-8). + printf => join('\r\n', ('HTTP/1.1 200 OK', '', '\0342\0234\0250\0377')), + # The raw bytes should come through as equally valued codepoints. They must not be decoded. + want => "HTTP/1.1 200 OK\n\n\xe2\x9c\xa8\xff", + }, +); + +for my $tc (@test_cases) { + @ddclient::curl = (@fakecurl, $tc->{printf}); + $ddclient::curl if 0; # suppress spurious warning "Name used only once: possible typo" + my $got = ddclient::geturl(url => 'http://ignored'); + is($got, $tc->{want}, $tc->{desc}); +} + +done_testing();