[PATCH] openggsn[master]: fix gsn_restart file buffer overflow and missing path sep

[Neels Hofmeyr]

Hello Jenkins Builder, Holger Freyther,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/1013

to look at the new patch set (#3).

fix gsn_restart file buffer overflow and missing path sep

Fix errors during gsn_restart file path composition:

- possible buffer overflow because the wrong remaining length was fed to
  strncat().
- missing path separator: put restart file in dir/gsn_restart instead of
  ../dirgsn_restart.

This assumes that the path separator is '/'.

Use talloc_asprintf() to fix all filename length problems and shorten the code.

In order to free the allocated path, add a free_filename label, and jump there
instead of returning from the fopen("w") failure branch. Also don't return from
"fclose failed" branch in order to free the path, remove the if {} braces.

Change-Id: Idf0a64ff45720aa818f2f9de1e8ba2fe2c82631b
---
M gtp/gtp.c
1 file changed, 7 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openggsn refs/changes/13/1013/3

diff --git a/gtp/gtp.c b/gtp/gtp.c
index 12cb492..ccb2611 100644
--- a/gtp/gtp.c
+++ b/gtp/gtp.c
@@ -646,13 +646,11 @@
 	FILE *f;
 	int i, rc;
 	int counter = 0;
-	char filename[NAMESIZE];
-
-	filename[NAMESIZE - 1] = 0;	/* No null term. guarantee by strncpy */
-	strncpy(filename, gsn->statedir, NAMESIZE - 1);
-	strncat(filename, RESTART_FILE, NAMESIZE - 1 - sizeof(RESTART_FILE));
+	char *filename;
 
 	i = umask(022);
+	filename = talloc_asprintf(NULL, "%s/%s", gsn->statedir, RESTART_FILE);
+	OSMO_ASSERT(filename);
 
 	/* We try to open file. On failure we will later try to create file */
 	if (!(f = fopen(filename, "r"))) {
@@ -680,17 +678,17 @@
 		LOGP(DLGTP, LOGL_ERROR,
 			"fopen(path=%s, mode=%s) failed: Error = %s\n", filename,
 			"w", strerror(errno));
-		return;
+		goto free_filename;
 	}
 
 	umask(i);
 	fprintf(f, "%d\n", gsn->restart_counter);
 close_file:
-	if (fclose(f)) {
+	if (fclose(f))
 		LOGP(DLGTP, LOGL_ERROR,
 			"fclose failed: Error = %s\n", strerror(errno));
-		return;
-	}
+free_filename:
+	talloc_free(filename);
 }
 
 int gtp_new(struct gsn_t **gsn, char *statedir, struct in_addr *listen,

-- 
To view, visit https://gerrit.osmocom.org/1013
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idf0a64ff45720aa818f2f9de1e8ba2fe2c82631b
Gerrit-PatchSet: 3
Gerrit-Project: openggsn
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>