ifconfig: Fix invalid free() in ifbridge

parse_vlans() does 's = strdup(str)', then calls strsep(&s, ...), then
attempts to free(s) at the end of the function.  For the success case,
this is fine (s is NULL, so it's a trivial memory leak), but in the
error case, we will attempt to free an invalid pointer.

Fix this by storing the original return value from strdup() and freeing
that instead.

MFC after:	3 seconds
Reported by:	David Gwynne <dlg@openbsd.org>
Reviewed by:	zlei, kevans
Sponsored by:	https://www.patreon.com/bsdivy
Differential Revision:	https://reviews.freebsd.org/D53545

1 files changed, 5 insertions, 3 deletions

diff --git a/sbin/ifconfig/ifbridge.c b/sbin/ifconfig/ifbridge.c
index eff443447c13..8bcf4a638adf 100644
--- a/sbin/ifconfig/ifbridge.c
+++ b/sbin/ifconfig/ifbridge.c
@@ -811,7 +811,7 @@ unsetbridge_private(if_ctx *ctx, const char *val, int dummy __unused)
 static int
 parse_vlans(ifbvlan_set_t *set, const char *str)
 {
-	char *s, *token;
+	char *s, *free_s, *token;
 
 	/* "none" means the empty vlan set */
 	if (strcmp(str, "none") == 0) {
@@ -829,6 +829,8 @@ parse_vlans(ifbvlan_set_t *set, const char *str)
 
 	if ((s = strdup(str)) == NULL)
 		return (-1);
+	/* Keep the original value of s, since strsep() will modify it */
+	free_s = s;
 
 	while ((token = strsep(&s, ",")) != NULL) {
 		unsigned long first, last;
@@ -856,11 +858,11 @@ parse_vlans(ifbvlan_set_t *set, const char *str)
 			BRVLAN_SET(set, vlan);
 	}
 
-	free(s);
+	free(free_s);
 	return (0);
 
 err:
-	free(s);
+	free(free_s);
 	return (-1);
 }