autofs-5.1.7 - fix concat_options() error handling

From: Ian Kent <raven@themaw.net>

There's a possibility of a memory leak in the mount options processing

when calling concat_options() in parse_mount() of the Sun format map

entry parsing.

There's also a case in do_init() of the Sun map format parsing where

a previously freed value is used in a logging statement without being

set to MULL.

So ensure concat_options() always frees it's arguments so that the

handling can be consistent in all places.

Signed-off-by: Ian Kent <raven@themaw.net>

---

 CHANGELOG           |    1 +

 modules/parse_sun.c |   24 +++++++++++-------------

 2 files changed, 12 insertions(+), 13 deletions(-)

--- autofs-5.1.7.orig/CHANGELOG

+++ autofs-5.1.7/CHANGELOG

@@ -77,6 +77,7 @@

 - fix lookup_prune_one_cache() refactoring change.

 - add missing description of null map option.

 - fix nonstrict offset mount fail handling.

+- fix concat_options() error handling.

 25/01/2021 autofs-5.1.7

 - make bind mounts propagation slave by default.

--- autofs-5.1.7.orig/modules/parse_sun.c

+++ autofs-5.1.7/modules/parse_sun.c

@@ -380,7 +380,8 @@ static int do_init(int argc, const char

 			if (!tmp) {

 				char *estr = strerror_r(errno, buf, MAX_ERR_BUF);

 				logerr(MODPREFIX "concat_options: %s", estr);

-				free(gbl_options);

+				/* freed in concat_options */

+				ctxt->optstr = NULL;

 			} else

 				ctxt->optstr = tmp;

 		} else {

@@ -492,12 +493,16 @@ static char *concat_options(char *left,

 	char *ret;

 	if (left == NULL || *left == '\0') {

+		if (!right || *right == '\0')

+			return NULL;

 		ret = strdup(right);

 		free(right);

 		return ret;

 	}

 	if (right == NULL || *right == '\0') {

+		if (left == NULL || *left == '\0')

+			return NULL;

 		ret = strdup(left);

 		free(left);

 		return ret;

@@ -508,6 +513,8 @@ static char *concat_options(char *left,

 	if (ret == NULL) {

 		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);

 		logerr(MODPREFIX "malloc: %s", estr);

+		free(left);

+		free(right);

 		return NULL;

 	}

@@ -989,14 +996,13 @@ static int parse_mapent(const char *ent,

 			if (newopt && strstr(newopt, myoptions)) {

 				free(myoptions);

 				myoptions = newopt;

-			} else {

+			} else if (newopt) {

 				tmp = concat_options(myoptions, newopt);

 				if (!tmp) {

 					char *estr;

 					estr = strerror_r(errno, buf, MAX_ERR_BUF);

 					error(logopt, MODPREFIX

 					      "concat_options: %s", estr);

-					free(myoptions);

 					return 0;

 				}

 				myoptions = tmp;

@@ -1358,16 +1364,12 @@ dont_expand:

 			if (mnt_options && noptions && strstr(noptions, mnt_options)) {

 				free(mnt_options);

 				mnt_options = noptions;

-			} else {

+			} else if (noptions) {

 				tmp = concat_options(mnt_options, noptions);

 				if (!tmp) {

 					char *estr = strerror_r(errno, buf, MAX_ERR_BUF);

 					error(ap->logopt,

 					      MODPREFIX "concat_options: %s", estr);

-					if (noptions)

-						free(noptions);

-					if (mnt_options)

-						free(mnt_options);

 					free(options);

 					free(pmapent);

 					return 1;

@@ -1387,15 +1389,11 @@ dont_expand:

 			if (options && mnt_options && strstr(mnt_options, options)) {

 				free(options);

 				options = mnt_options;

-			} else {

+			} else if (mnt_options) {

 				tmp = concat_options(options, mnt_options);

 				if (!tmp) {

 					char *estr = strerror_r(errno, buf, MAX_ERR_BUF);

 					error(ap->logopt, MODPREFIX "concat_options: %s", estr);

-					if (options)

-						free(options);

-					if (mnt_options)

-						free(mnt_options);

 					free(pmapent);

 					return 1;

 				}