From ff30f98e6a1ea1de5ed4ea69cc3552440ee7f26c Mon Sep 17 00:00:00 2001 From: Ralph Corderoy Date: Tue, 23 Jan 2018 16:37:01 +0000 Subject: [PATCH] fmt_scan(): Avoid undefined behaviour from overlapping strncpy(3). 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 | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sbr/fmt_scan.c b/sbr/fmt_scan.c index 67f0a03e..091ba75d 100644 --- a/sbr/fmt_scan.c +++ b/sbr/fmt_scan.c @@ -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: -- 2.56.0