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/ddclient.in b/ddclient.in index 6b32de4..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 $curl = 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", @@ -2657,7 +2658,7 @@ sub curl_cmd { ); debug("CURL: %s", $curl); - fatal("curl not found") if ($curl eq ''); + 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 @@ -2674,8 +2675,15 @@ sub curl_cmd { } close($tfh); # 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. - open(my $cfh, '-|', $curl, '--config', $tmpfile) or fatal("failed to run curl ($curl): $!"); + # 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) { 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();