[openggsn] [PATCH 1/2] Fix possible buffer overflow for gsn_restart file path

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Neels Hofmeyr nhofmeyr at sysmocom.de
Thu Aug 18 02:06:14 UTC 2016


For strncat, to obtain n, one must not subtract the length of what is appended,
but the length of what is already written from the buffer size.

Verified with this little test program:

 #include <stdio.h>
 #include <string.h>

 int main() {
   char buf[20];
   strncpy(buf, "123", 10);
   strncat(buf, "456789012345", 10 - strlen(buf));
   printf("%s\n", buf);
   return 0;
 }

It prints "1234567890".
---
 gtp/gtp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gtp/gtp.c b/gtp/gtp.c
index 12cb492..34e1dc6 100644
--- a/gtp/gtp.c
+++ b/gtp/gtp.c
@@ -648,9 +648,10 @@ static void log_restart(struct gsn_t *gsn)
 	int counter = 0;
 	char filename[NAMESIZE];
 
-	filename[NAMESIZE - 1] = 0;	/* No null term. guarantee by strncpy */
+	/* guarantee nul term, strncpy might omit if too long */
+	filename[NAMESIZE - 1] = 0;
 	strncpy(filename, gsn->statedir, NAMESIZE - 1);
-	strncat(filename, RESTART_FILE, NAMESIZE - 1 - sizeof(RESTART_FILE));
+	strncat(filename, RESTART_FILE, NAMESIZE - 1 - strlen(filename));
 
 	i = umask(022);
 
-- 
2.1.4




More information about the OpenBSC mailing list