]> diplodocus.org Git - nmh/commitdiff
fmt_scan(): Avoid undefined behaviour from overlapping strncpy(3).
authorRalph Corderoy <ralph@inputplus.co.uk>
Tue, 23 Jan 2018 16:37:01 +0000 (16:37 +0000)
committerRalph Corderoy <ralph@inputplus.co.uk>
Tue, 23 Jan 2018 18:05:33 +0000 (18:05 +0000)
M. Levinson pointed out in private email that pcc(1) tickles a bug that
causes undefined behaviour due to part of `buffer[]', accessed by `str',
being strncpy(3) back to its start.  Fix by using memmove(3) to slide
`str' back to `buffer's beginning whenever it may have wandered off.

Fixes e8635a8a that added `%(unquote)', but using `%(trim)' twice was
also faulty in the same way and pre-dates git.

sbr/fmt_scan.c

index 67f0a03..091ba75 100644 (file)
@@ -413,7 +413,10 @@ fmt_scan (struct format *format, charstring_t scanlp, int width, int *dat,
          struct fmt_callbacks *callbacks)
 {
     char *sp;
-    char *savestr, *str;
+    /* If str points to part of buffer[] or buffer2[] then it must only
+     * ever point at their first element as otherwise undefined
+     * behaviour from overlapping strncpy(3)s can result. */
+    char *str, *savestr;
     char buffer[NMH_BUFSIZ], buffer2[NMH_BUFSIZ];
     int i, c;
     bool rjust;
@@ -757,6 +760,7 @@ fmt_scan (struct format *format, charstring_t scanlp, int width, int *dat,
                            *xp-- = '\0';
                    if (rjust && i > 0 && (int) strlen(str) > i)
                        str += strlen(str) - i;
+                    str = memmove(buffer, str, strlen(str) + 1);
            }
            break;
 
@@ -946,6 +950,7 @@ fmt_scan (struct format *format, charstring_t scanlp, int width, int *dat,
                            else
                                break;
                    }
+                    str = memmove(buffer, str, strlen(str) + 1);
                } else if (!(str = get_x400_friendly (mn->m_mbox,
                                buffer, sizeof(buffer)))) {
        unfriendly: