Fix unsafe string manipulations in XS code

Fix two instances of unsafe C string manipulations, vulnerable to null
pointer dereferences and out-of-bounds accesses in edge cases.

This was observed as segfaults in zonemaster-cli when attempting to
process the following malformed resource record:

  bad-caa.example.   IN  CAA  \# 4 C0000202

Zonemaster::LDNS::RR and Zonemaster::LDNS::Packet objects can be
converted to a string (i.e. presentation format) with the string()
method. Doing so triggers a call to the ldns_rr2str() and ldns_pkt2str()
C functions respectively.

However, when given some classes of malformed packets, ldns’s functions
fail by returning NULL instead of a valid C string. Normally, these
strings end with a newline, which is removed in the XS code before
returning the result. But the removal of that newline character is
attempted without checking for NULL pointers or empty strings.

With this commit, Zonemaster::LDNS::RR->new() will now croak when given
the aforementioned malformed resource record, and so will
Zonemaster::LDNS::Packet->string() if it contains such a resource
record.

3 files changed, 34 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 becb907..8f704c8 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:
@@ -1597,7 +1604,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';
+    }
+}