linux-kernel - [PATCH] tipc: avoid possible string overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 28 Mar 2018 16:02:04 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Jon Maloy <jon.maloy@...csson.com>,
        Ying Xue <ying.xue@...driver.com>,
        "David S. Miller" <davem@...emloft.net>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@...csson.com>,
        netdev@...r.kernel.org, tipc-discussion@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org
Subject: [PATCH] tipc: avoid possible string overflow

gcc points out that the combined length of the fixed-length inputs to
l->name is larger than the destination buffer size:

net/tipc/link.c: In function 'tipc_link_create':
net/tipc/link.c:465:26: error: '%s' directive writing up to 32 bytes into a region of size between 26 and 58 [-Werror=format-overflow=]
  sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
                          ^~                              ~~~~~~~~
net/tipc/link.c:465:2: note: 'sprintf' output 11 or more bytes (assuming 75) into a destination of size 60
  sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);

Using snprintf() ensures that the destination is still a nul-terminated
string in all cases. It's still theoretically possible that the string
gets trunctated though, so this patch should be carefully reviewed to
ensure that either truncation is impossible in practice, or that we're
ok with the truncation.

Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash values")
Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
 net/tipc/link.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 1289b4ba404f..c195ba036035 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -462,7 +462,8 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id,
 			sprintf(peer_str, "%x", peer);
 	}
 	/* Peer i/f name will be completed by reset/activate message */
-	sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
+	snprintf(l->name, sizeof(l->name), "%s:%s-%s:unknown",
+		 self_str, if_name, peer_str);
 
 	strcpy(l->if_name, if_name);
 	l->addr = peer;
-- 
2.9.0