[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

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

[Steve Dickson]

On 9/3/26 1:53 PM, Scott Mayhew wrote:
> 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>
Committed... (tag: nfs-utils-2-9-3-rc4)

steved.
> ---
>   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;

[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

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

[Steve Dickson]

On 9/3/26 1:53 PM, Scott Mayhew wrote:
> 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>
Committed... (tag: nfs-utils-2-9-3-rc4)

steved.
> ---
>   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;