Wednesday, January 01, 2020

Re: Probable off by one in src/usr.bin/rdist/docmd.c

On Wed, Jan 01, 2020 at 04:02:24PM +0100, Aham Brahmasmi wrote:

> Namaste misc,
>
> Question:
> In the makeconn function in src/usr.bin/rdist/docmd.c, should the 5 in
> the following line be replaced by 4?
> ...
> static int
> makeconn(char *rhost)
> {
> ...
> (void) snprintf(buf, sizeof(buf), "%.*s -S",
> (int)(sizeof(buf)-5), path_rdistd);
> ...
>
> Explanation:
> I have a limited ability to read code, so I may be wrong here.
>
> If I am not wrong, strings are terminated with '\0' which I think is a
> single byte. So, in the above case, the sizeof(" -S" + '\0')=4. But the
> code has 5.
>
> I am not sure of my "'\0' is a single byte" part, and hence my query.

By definition, '\0' is a single byte. sizeof(String literal) included
the terminating '\0'. So sizeof("foo") is 4.

The sizeof(buf)-5 fills in the precision of the %....s. That means
that path_rdistd wil be limited to that number of chars. The " -S"
part indeed takes 3 chars, so there is sizeof(buf) - 3 left for
path_rdistd, excluding the terminating '\0'. So -4 is indeed right.

Butt does it matter? I'd say no, only if path_rdistd is close to
BUFSIZ in length tunrcation will happen 1 char earlier than possible.
I would argue that specifying the precision here is rather confusing,
and it would be better to use the standard idiom equivalent to the
example in the snprintf man page.

-Otto

No comments:

Post a Comment