[nfs-utils PATCH 0/2] Two hardening fixes for junction handling

[nfs-utils PATCH 0/2] Two hardening fixes for junction handling

[Scott Mayhew]

The first patch fixes a buffer overrun in junction_parse_xml_read()
and the second fixes a use-after-free when parsing multi-location
junctions in locations_to_fslocdata().

Scott Mayhew (2):
  junction: fix buffer over-read in junction_parse_xml_read()
  junction: fix use-after-free in locations_to_fslocdata()

 support/export/cache.c      | 6 ++++++
 support/junction/junction.c | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.55.0

[nfs-utils PATCH 1/2] junction: fix buffer over-read in junction_parse_xml_read()

[Scott Mayhew]

junction_get_xattr() returns an opaque buffer, but
junction_parse_xml_read() treats it as a string by passing it to xlog().
Fix it by allocating an extra byte and null-terminating the buffer in
junction_get_xattr().

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 support/junction/junction.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/support/junction/junction.c b/support/junction/junction.c
index c1ec8ff8..04a8ebf1 100644
--- a/support/junction/junction.c
+++ b/support/junction/junction.c
@@ -293,7 +293,7 @@ junction_get_xattr(int fd, const char *path, const char *name, void **contents,
 		return FEDFS_ERR_ACCESS;
 	}
 
-	xattrbuf = malloc((size_t)len);
+	xattrbuf = malloc((size_t)len + 1);
 	if (xattrbuf == NULL) {
 		xlog(D_GENERAL, "%s: failed to get buffer for xattr %s on %s",
 			__func__, name, path);
@@ -307,6 +307,7 @@ junction_get_xattr(int fd, const char *path, const char *name, void **contents,
 		return FEDFS_ERR_ACCESS;
 	}
 
+	((char *) xattrbuf)[len] = '\0';
 	xlog(D_CALL, "%s: read xattr %s from path %s",
 			__func__, name, path);
 	*contents = xattrbuf;
-- 
2.55.0

[nfs-utils PATCH 2/2] junction: fix use-after-free in locations_to_fslocdata()

[Scott Mayhew]

While parsing a multi-location junction, locations_to_fslocdata() sets
last_path = root_path and then frees root_path, leaving last_path
dangling.  On the next iteration, strcmp(rootpath, last_path)
dereferences freed memory.

Fix it by transferring ownership of root_path to last_path when they
should be aliased.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 support/export/cache.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/support/export/cache.c b/support/export/cache.c
index 059f48a7..9f71c1dd 100644
--- a/support/export/cache.c
+++ b/support/export/cache.c
@@ -2806,19 +2806,25 @@ static bool locations_to_fslocdata(struct nfs_fsloc_set *locations,
 			}
 			remaining -= (size_t)len;
 			ptr += len;
+			free(last_path);
 			last_path = rootpath;
+			rootpath = NULL;
 		}
 
 		seen = true;
 		free(rootpath);
+		rootpath = NULL;
 		free(server);
+		server = NULL;
 	}
 
+	free(last_path);
 	xlog(D_CALL, "%s: fslocdata='%s', ttl=%d",
 		__func__, fslocdata, *ttl);
 	return seen;
 
 out_false:
+	free(last_path);
 	free(rootpath);
 	free(server);
 	return false;
-- 
2.55.0