Re: [PATCH 1/4] conf: Copy up to MAXDNSRCH - 1 bytes, not MAXDNSRCH

27 Jun 2024

On Thu, Jun 27, 2024 at 09:27:01AM +0200, Stefano Brivio wrote:
...
On Thu, 27 Jun 2024 10:45:28 +1000
David Gibson  wrote:
...
On Thu, Jun 27, 2024 at 01:45:33AM +0200, Stefano Brivio wrote:
...
Spotted by Coverity just recently. Not that it really matters as
MAXDNSRCH always appears to be defined as 1025, while a full domain
name can have up to 253 characters: it would be a bit pointless to
have a longer search domain.
Signed-off-by: Stefano Brivio 
Hm.  So, IIRC strncpy() won't \0 terminate in the case where it
truncates.  I guess we'll get away with that here since we expect
c->dns_search to be filled with \0 before hand.  That's... more
fragile than ideal, though.
Well, we know we start from a zero-initialised area, that's by design,
it's not that we get away with it. Without that consideration not many
things would work in this function.
That's a fair point.

Reviewed-by: David Gibson 
...
Are you suggesting to use snprintf()? It looks a bit pedantic to me but
I'm fine with it. Otherwise, feel free to post a patch fixing it in a
way you feel it's ideal...
...
...
---
 conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/conf.c b/conf.c
index e1f5422..9e47e9a 100644
--- a/conf.c
+++ b/conf.c
@@ -453,7 +453,7 @@ static void get_dns(struct ctx *c)
      	while (s - c->dns_search < ARRAY_SIZE(c->dns_search) - 1
      	       /* cppcheck-suppress strtokCalled */
      	       && (p = strtok(NULL, " \t"))) {
-				strncpy(s->n, p, sizeof(c->dns_search[0]));
+				strncpy(s->n, p, sizeof(c->dns_search[0]) - 1);
      		s++;
      		*s->n = 0;
      	}
-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

    