diff options
author | Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com> | 2022-11-28 07:58:01 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-11-28 07:58:01 +0100 |
commit | 1562cd139a61ffd51656961d938b1b47d23e090a (patch) | |
tree | 1d1ba57aa5802aa860593eab132b49b70cde9316 | |
parent | 1fd032c2c914bb9c6c964fef1eec8e6998032a4b (diff) | |
parent | 65a7ce9e5d5faba1926b25bb2f173951b55fcd08 (diff) |
Merge pull request #153 from marc-vanderwal/bugfix/#149
Fix unsafe string manipulations in XS code
-rw-r--r-- | include/LDNS.h | 1 | ||||
-rw-r--r-- | src/LDNS.xs | 18 | ||||
-rw-r--r-- | src/assist.c | 17 | ||||
-rw-r--r-- | t/packet.t | 31 | ||||
-rw-r--r-- | t/rr.t | 11 |
5 files changed, 76 insertions, 2 deletions
diff --git a/include/LDNS.h b/include/LDNS.h index 9de985a..813b00e 100644 --- a/include/LDNS.h +++ b/include/LDNS.h @@ -108,6 +108,7 @@ typedef ldns_rr *Zonemaster__LDNS__RR__X25; SV *rr2sv(ldns_rr *rr); char *randomize_capitalization(char *in); +void strip_newline(char* in); #ifdef USE_ITHREADS void net_ldns_remember_resolver(SV *rv); diff --git a/src/LDNS.xs b/src/LDNS.xs index 822018a..551a3ba 100644 --- a/src/LDNS.xs +++ b/src/LDNS.xs @@ -1233,7 +1233,14 @@ packet_string(obj) Zonemaster::LDNS::Packet obj; CODE: RETVAL = ldns_pkt2str(obj); - RETVAL[strlen(RETVAL)-1] = '\0'; + if(RETVAL == NULL || RETVAL[0] == '\0') + { + croak("Failed to convert packet to string"); + } + else + { + strip_newline(RETVAL); + } OUTPUT: RETVAL CLEANUP: @@ -1667,7 +1674,14 @@ rr_string(obj) Zonemaster::LDNS::RR obj; CODE: RETVAL = ldns_rr2str(obj); - RETVAL[strlen(RETVAL)-1] = '\0'; + if(RETVAL == NULL || RETVAL[0] == '\0') + { + croak("Failed to convert RR to string"); + } + else + { + strip_newline(RETVAL); + } OUTPUT: RETVAL CLEANUP: diff --git a/src/assist.c b/src/assist.c index 3f52792..413761e 100644 --- a/src/assist.c +++ b/src/assist.c @@ -228,3 +228,20 @@ rr2sv(ldns_rr *rr) return rr_sv; } + +void +strip_newline(char* in) +{ + size_t length; + + if (in == NULL || in[0] == '\0') + { + return; + } + + length = strlen(in); + if (in[length - 1] == '\n') + { + in[length - 1] = '\0'; + } +} @@ -28,4 +28,35 @@ is($p->answerfrom, undef, 'No answerfrom'); $p->answerfrom('127.0.0.1'); is($p->answerfrom, '127.0.0.1', 'Localhost'); +subtest "croak when stringifying packet with malformed CAA" => sub { + my $will_croak = sub { + # Constructing a synthetic packet that would have the following string + # representation in dig-like format: + # + # ;; ->>HEADER<<- opcode: QUERY, rcode: NOERROR, id: 13944 + # ;; flags: qr aa ; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 0 + # ;; QUESTION SECTION: + # ;; bad-caa.example. IN CAA + # + # ;; ANSWER SECTION: + # bad-caa.example. 3600 IN CAA \# 4 C0000202 + # + # ;; AUTHORITY SECTION: + # + # ;; ADDITIONAL SECTION: + my $packet_bin = pack( + 'H*', + '367884000001000100000000' . # header + '076261642d636161076578616d706c650001010001' . # question + 'c00c0101000100000e100004c0000202' # bad answer + ); + + my $packet = Zonemaster::LDNS::Packet->new_from_wireformat( $packet_bin ); + + # This must croak + $packet->string; + }; + like( exception { $will_croak->() }, qr/^Failed to convert packet to string/ ); +}; + done_testing(); @@ -235,4 +235,15 @@ subtest 'SPF' => sub { }; +subtest 'croak when given malformed CAA records' => sub { + my $will_croak = sub { + # This will croak if LDNS.xs is compiled with -DUSE_ITHREADS + my $bad_caa = Zonemaster::LDNS::RR->new( + 'bad-caa.example. 3600 IN CAA \# 4 C0000202' ); + # This will always croak + $bad_caa->string(); + }; + like( exception { $will_croak->() }, qr/^Failed to convert RR to string/ ); +}; + done_testing; |