From 6b83413b34784eb3956c026c3b955e532f7f02bc Mon Sep 17 00:00:00 2001 From: Rob Davies Date: Wed, 10 Jan 2018 17:05:57 +0000 Subject: [PATCH] Fix undefined behaviour and sign extension issues in kstrtok Prevent read beyond the end of sep if it's an empty string. Change to internally use unsigned char to avoid unwanted sign extension on platforms where 'char' is signed. Fix undefined behaviour where pointers were made to the element before the start of `str`: https://www.securecoding.cert.org/confluence/display/c/ARR30-C.+Do+not+form+or+use+out-of-bounds+pointers+or+array+subscripts --- kstring.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/kstring.c b/kstring.c index 3e0b642..aa6c2ee 100644 --- a/kstring.c +++ b/kstring.c @@ -34,28 +34,29 @@ int ksprintf(kstring_t *s, const char *fmt, ...) return l; } -char *kstrtok(const char *str, const char *sep, ks_tokaux_t *aux) +char *kstrtok(const char *str, const char *sep_in, ks_tokaux_t *aux) { - const char *p, *start; + const unsigned char *p, *start, *sep = (unsigned char *) sep_in; if (sep) { // set up the table if (str == 0 && aux->finished) return 0; // no need to set up if we have finished aux->finished = 0; - if (sep[1]) { + if (sep[0] && sep[1]) { aux->sep = -1; aux->tab[0] = aux->tab[1] = aux->tab[2] = aux->tab[3] = 0; for (p = sep; *p; ++p) aux->tab[*p>>6] |= 1ull<<(*p&0x3f); } else aux->sep = sep[0]; } if (aux->finished) return 0; - else if (str) aux->p = str - 1, aux->finished = 0; + else if (str) start = (unsigned char *) str, aux->finished = 0; + else start = (unsigned char *) aux->p + 1; if (aux->sep < 0) { - for (p = start = aux->p + 1; *p; ++p) + for (p = start; *p; ++p) if (aux->tab[*p>>6]>>(*p&0x3f)&1) break; } else { - for (p = start = aux->p + 1; *p; ++p) + for (p = start; *p; ++p) if (*p == aux->sep) break; } - aux->p = p; // end of token + aux->p = (const char *) p; // end of token if (*p == 0) aux->finished = 1; // no more tokens return (char*)start; }