Merge pull request #177 from marc-vanderwal/bugfix/nsec3-salt-method

Fix crashing induced by Zonemaster::LDNS::RR::NSEC3::salt() method
zonemaster · Nov 21, 2023 · d079e46 · d079e46
2 parents 30403e1 + b8f44fe
commit d079e46
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 11 deletions.
diff --git a/lib/Zonemaster/LDNS/RR/NSEC3.pm b/lib/Zonemaster/LDNS/RR/NSEC3.pm
@@ -37,7 +37,7 @@ Returns the iteration count.
 
 =item salt()
 
-Returns the cryptographic salt, in binary form.
+Returns the contents of the salt field as a binary string, if non-empty; otherwise, returns an empty string. If there was a problem accessing the salt field, returns undef.
 
 =item next_owner()
 

diff --git a/src/LDNS.xs b/src/LDNS.xs
@@ -2275,13 +2275,16 @@ rr_nsec3_iterations(obj)
 SV *
 rr_nsec3_salt(obj)
     Zonemaster::LDNS::RR::NSEC3 obj;
-    PPCODE:
-        if(ldns_nsec3_salt_length(obj) > 0)
-        {
-            ldns_rdf *buf = ldns_nsec3_salt(obj);
-            ST(0) = sv_2mortal(newSVpvn((char *)ldns_rdf_data(buf), ldns_rdf_size(buf)));
-            ldns_rdf_deep_free(buf);
+    CODE:
+    {
+        uint8_t *salt = ldns_nsec3_salt_data(obj);
+        if (salt) {
+            RETVAL = newSVpvn((char *)salt, ldns_nsec3_salt_length(obj));
+            LDNS_FREE(salt);
         }
+    }
+    OUTPUT:
+        RETVAL
 
 SV *
 rr_nsec3_next_owner(obj)

diff --git a/t/rr.t b/t/rr.t
@@ -203,28 +203,43 @@ subtest 'DS' => sub {
     }
 };
 
-subtest 'NSEC3' => sub {
+subtest 'NSEC3 without salt' => sub {
     my $nsec3 = Zonemaster::LDNS::RR->new_from_string(
         'VD0J8N54V788IUBJL9CN5MUD416BS5I6.com. 86400 IN NSEC3 1 1 0 - VD0N3HDL5MG940MOUBCF5MNLKGDT9RFT NS DS RRSIG' );
     isa_ok( $nsec3, 'Zonemaster::LDNS::RR::NSEC3' );
     is( $nsec3->algorithm, 1 );
     is( $nsec3->flags,     1 );
     ok( $nsec3->optout );
     is( $nsec3->iterations,                  0 );
-    is( $nsec3->salt,                        undef );
-    is( encode_base64( $nsec3->next_owner ), "FPtBccW1LaCSAtjy2PLa9aQb1O39\n" );
+    is( $nsec3->salt,                        '' );
+    is( encode_base64( $nsec3->next_owner, '' ), "FPtBccW1LaCSAtjy2PLa9aQb1O39" );
     is( $nsec3->typelist,                    'NS DS RRSIG ' );
 
     is_deeply( [ sort keys %{ $nsec3->typehref } ], [qw(DS NS RRSIG)] );
 };
 
+subtest 'NSEC3 with salt' => sub {
+    my $nsec3 = Zonemaster::LDNS::RR->new_from_string(
+        'BP7OICBR09FICEULBF46U8DMJ1J1V8R3.bad-values.dnssec03.xa. 900 IN NSEC3 2 1 1 8104 c91qe244nd0q5qh3jln35a809mik8d39 A NS SOA MX TXT RRSIG DNSKEY NSEC3PARAM' );
+    isa_ok( $nsec3, 'Zonemaster::LDNS::RR::NSEC3' );
+    is( $nsec3->algorithm, 2 );
+    is( $nsec3->flags,     1 );
+    ok( $nsec3->optout );
+    is( $nsec3->iterations,                  1 );
+    is( unpack('H*', $nsec3->salt),          '8104' );
+    is( encode_base64( $nsec3->next_owner, '' ), "FGJDpwiEu0Gi6iOdbjKpAE2lRDRp" );
+    is( $nsec3->typelist,                    'A NS SOA MX TXT RRSIG DNSKEY NSEC3PARAM ' );
+
+    is_deeply( [ sort keys %{ $nsec3->typehref } ], [qw(A DNSKEY MX NS NSEC3PARAM RRSIG SOA TXT)] );
+};
+
 subtest 'NSEC3PARAM' => sub {
     my $nsec3param = Zonemaster::LDNS::RR->new_from_string( 'whitehouse.gov.		3600	IN	NSEC3PARAM 1 0 1 B2C19AB526819347' );
     isa_ok( $nsec3param, 'Zonemaster::LDNS::RR::NSEC3PARAM' );
     is( $nsec3param->algorithm,  1 );
     is( $nsec3param->flags,      0 );
     is( $nsec3param->iterations, 1, "Iterations" );
-    is( encode_base64( $nsec3param->salt ), "CLLBmrUmgZNH\n", "Salt" );
+    is( encode_base64( $nsec3param->salt, '' ), "CLLBmrUmgZNH", "Salt" );
     is( lc($nsec3param->owner), 'whitehouse.gov.' );
 };