Compiling master with GCC gives: screen.c: In function ‘main’: screen.c:794:56: warning: ‘sprintf’ may write a terminating nul past the end of the destination [-Wformat-overflow=] 794 | sprintf(SocketPath, "%s/.screen", home); | ^ screen.c:794:25: note: ‘sprintf’ output between 9 and 4097 bytes into a destination of size 4096 794 | sprintf(SocketPath, "%s/.screen", home); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Indeed, the test strlen(home) > MAXPATHLEN - 8 is not sufficient due to the terminating null character (if home has length MAXPATHLEN - 8, then MAXPATHLEN + 1 characters are written, which is larger than the buffer size MAXPATHLEN). The attached patch fixes this problem and makes the warning disappear. -- Vincent Lefèvre <vinc...@vinc17.net> - Web: <https://www.vinc17.net/> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
>From bb2aa72172beb87a9f12660e76395269683861c6 Mon Sep 17 00:00:00 2001 From: Vincent Lefevre <vinc...@vinc17.net> Date: Mon, 8 Jul 2024 15:38:16 +0200 Subject: [PATCH] Really avoid a potential buffer overflow for 'home' This fixes commit b4e5968f0b7ad7673a57c11b91f359139418f4c0, taking the terminating null character into account. --- src/screen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/screen.c b/src/screen.c index 44f6a15..b5d91a6 100644 --- a/src/screen.c +++ b/src/screen.c @@ -789,7 +789,7 @@ int main(int argc, char **argv) } else { #ifndef SOCKET_DIR if (SocketDir == NULL) { - if (strlen(home) > MAXPATHLEN - 8) + if (strlen(home) > MAXPATHLEN - 8 - 1) Panic(0, "$HOME too long - sorry."); sprintf(SocketPath, "%s/.screen", home); SocketDir = SocketPath; -- 2.45.2