Bug 495 - dname_to_string_r split broke strlcpy with label_count==1
dname_to_string_r split broke strlcpy with label_count==1
Status: RESOLVED FIXED
Product: NSD
Classification: Unclassified
Component: NSD Code
3.2.x
All OpenBSD
: P5 normal
Assigned To: NSD team
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-29 11:43 CET by stu-nlnetlabs
Modified: 2013-04-03 11:36 CEST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description stu-nlnetlabs 2013-03-29 11:43:57 CET
r3481 (commit log is just "secure64 patch") of nsd3 split the dname_to_string function in two; diff of this part of the commit is below for ease of reference.

--snip--
@@ -382,16 +392,27 @@
 dname_to_string(const dname_type *dname, const dname_type *origin)
 {
        static char buf[MAXDOMAINLEN * 5];
+       return dname_to_string_r(dname, origin, buf);
+}
+
+const char *
+dname_to_string_r(const dname_type *dname, const dname_type *origin,
+       char* buf)
+{
        size_t i;
-       size_t labels_to_convert = dname->label_count - 1;
+       size_t labels_to_convert = 0;
        int absolute = 1;
        char *dst;
        const uint8_t *src;
-
+       if (!dname) {
+               *buf = '\0';
+               return buf;
+       }
        if (dname->label_count == 1) {
                strlcpy(buf, ".", sizeof(buf));
                return buf;
        }
+       labels_to_convert = dname->label_count - 1;
 
        if (origin && dname_is_subdomain(dname, origin)) {
                int common_labels = dname_label_match_count(dname, origin);

--snip--

... so now buf[] is defined in dname_to_string but sizeof(buf) is used in dname_to_string_r, returning an incorrect value of 1. as a result, in the dname->label_count==1 case, there is only space for the NUL, so buf is now an empty string rather than a ".".
Comment 1 Matthijs Mekking 2013-04-03 11:36:39 CEST
Hi, 

Thanks for your report. I believe in most cases this is not broken, as the size of a char pointer is usually 4 bytes (on 32bit systems). Nevertheless, you are right that it is not the intended size and so I fixed it for the next NSD release.

Best regards,
  Matthijs