On Wed, 26 Oct 2016, Paul Goyette wrote:
HOWEVER,
Looking at dev_untokenstring() it would appear that there is a potential
problem there! We have
cp = buf + strlcat(buf, words + *token, len - 2);
cp[0] = ' ';
cp[1] = '\0';
Since strlcat(3) (quoting the man page) "return[s] the total length of the
string [it] tried to create", it is entirely possible for cp to point beyond
the end of the buffer. Thus the insertion of a trailing space-between-word
and the NUL character could occur on some random location.
It would seem to me the this code should be written as
newlen = strlcat(buf, word + *token, len - 2);
if (newlen > len - 2)
newlen = len - 2;
cp = buf + newlen;
cp[0] = ' ';
cp[1] = '\0';
I have confirmed that this actually is the root cause of the stack
overflow problem.
With this fix, and the original buffer size of 64, I get a truncated
product description (as expected), but the stack overflow is gone.
With this fix and the updated buffer size, I get the full description
without any overflow.
With the original code (smaller buffer, above fix not included) I got
consistent stack overflow instead of proper printing of the product
description.
The above fix has been committed.
+------------------+--------------------------+------------------------+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| (Retired) | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+------------------+--------------------------+------------------------+