On Fri, Apr 18, 2014 at 05:19:15PM -0700, Claus Assmann wrote:
> Seems it is ok to use strlcat/strlcpy that way in some cases:
> $ cat src/usr.sbin/smtpd/*.c | egrep -c ' strlc(at|py)\('
> 249
> 

We tend to be very strict with our checks in smtpd and we did not check
in various places because the copies occur between buffers of same size
or because length is checked and handled earlier or later.

However, your mail prompted me into having an audit, and so I spent the
whole day looking at every single call to make sure we didn't miss some
truncation check somewhere.

I started by removing all void casts to make sure I did not assume some
were ok just because they were previously marked as such.

I then proceeded to casting void the easy and obvious ones, for example
those that copy constant strings to larger buffers, the ones that can't
overflow because of former checks or identical buffer size copies, ...
This was the most common case.

In some cases, it was a bit less obvious as the buffer was coming from
another process through imsg, I made sure the assumptions were correct
and void casted those too when I was 100% sure they were safe and that
the code path was trivial to follow.

In a couple cases I did add checks even though it was safe because the
code path was a bit tricky and it seemed like the check was not taking
place at the appropriate place (best example is rev 1.15 of table.c).
In places where it's safe because two buffers use different defines of
same value, I also added checks just in case we ever change one of the
values without paying attention to the consequences.

Finally, there were some instances where we did miss a check that would
later lead to a failure with an inappropriate error message (ie: if the
interface name exceeded the size of a buffer, we would fail in an ioctl
call a bit later rather than right away).

Anyways, all calls are now checked and you can review the commits which
were done today, I made it clear in every commit log was fixed.

NOW IS TIME FOR WINE AND CHEEZE.

-- 
Gilles Chehade

https://www.poolp.org                                          @poolpOrg

Reply via email to