geturl: Set raw (binary) mode when reading from curl

This commit is contained in:
Richard Hansen 2024-05-26 02:24:53 -04:00
parent 8e901c3db6
commit 31dbd8e4ed
3 changed files with 40 additions and 4 deletions

View file

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

View file

@ -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 <https://perldoc.perl.org/PerlIO#Alternatives-to-raw>, 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) {

27
t/geturl_response.pl Normal file
View file

@ -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();