[PATCH v2] libnsm: fix the safer atomic filenames fix

[PATCH v2] libnsm: fix the safer atomic filenames fix

[Benjamin Coddington]

Commit 9f7a91b51ffc ("libnsm: safer atomic filenames") messed up the length
arguement to snprintf() in nsm_make_temp_pathname such that the length is
longer than the computed string.  When compiled with "-O
-D_FORTIFY_SOURCE=3", __snprintf_chk will fail and abort statd.

The fix is to correct the original size calculation, then pull one from the
snprintf length for the final "/".

Fixes: 9f7a91b51ffc ("libnsm: safer atomic filenames")
Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
v2: ensure we handle paths without '/', simplify.

---
 support/nsm/file.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/support/nsm/file.c b/support/nsm/file.c
index e0804136ccbe..de122b0fc5a1 100644
--- a/support/nsm/file.c
+++ b/support/nsm/file.c
@@ -187,7 +187,7 @@ nsm_make_temp_pathname(const char *pathname)
 	char *path, *base;
 	int len;
 
-	size = strlen(pathname) + sizeof(".new") + 3;
+	size = strlen(pathname) + sizeof(".new") + 1;
 	if (size > PATH_MAX)
 		return NULL;
 
@@ -196,15 +196,19 @@ nsm_make_temp_pathname(const char *pathname)
 		return NULL;
 
 	base = strrchr(pathname, '/');
-	strcpy(path, pathname);
+	if (base == NULL)
+		base = pathname;
+	else
+		base++;
 
+	strcpy(path, pathname);
 	len = base - pathname;
-	len += snprintf(path + len + 1, size-len, ".%s.new", base+1);
+	len += snprintf(path + len, size - len, ".%s.new", base);
+
 	if (error_check(len, size)) {
 		free(path);
 		return NULL;
 	}
-
 	return path;
 }
 

base-commit: eb5abb5c60ab60f3c2f22518d513ed187f39bd9b
-- 
2.47.0

Re: [PATCH v2] libnsm: fix the safer atomic filenames fix

[Steve Dickson]

On 11/27/24 6:44 AM, Benjamin Coddington wrote:
> Commit 9f7a91b51ffc ("libnsm: safer atomic filenames") messed up the length
> arguement to snprintf() in nsm_make_temp_pathname such that the length is
> longer than the computed string.  When compiled with "-O
> -D_FORTIFY_SOURCE=3", __snprintf_chk will fail and abort statd.
> 
> The fix is to correct the original size calculation, then pull one from the
> snprintf length for the final "/".
> 
> Fixes: 9f7a91b51ffc ("libnsm: safer atomic filenames")
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
Reverted and Committed....

steved.

> ---
> v2: ensure we handle paths without '/', simplify.
> 
> ---
>   support/nsm/file.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/support/nsm/file.c b/support/nsm/file.c
> index e0804136ccbe..de122b0fc5a1 100644
> --- a/support/nsm/file.c
> +++ b/support/nsm/file.c
> @@ -187,7 +187,7 @@ nsm_make_temp_pathname(const char *pathname)
>   	char *path, *base;
>   	int len;
>   
> -	size = strlen(pathname) + sizeof(".new") + 3;
> +	size = strlen(pathname) + sizeof(".new") + 1;
>   	if (size > PATH_MAX)
>   		return NULL;
>   
> @@ -196,15 +196,19 @@ nsm_make_temp_pathname(const char *pathname)
>   		return NULL;
>   
>   	base = strrchr(pathname, '/');
> -	strcpy(path, pathname);
> +	if (base == NULL)
> +		base = pathname;
> +	else
> +		base++;
>   
> +	strcpy(path, pathname);
>   	len = base - pathname;
> -	len += snprintf(path + len + 1, size-len, ".%s.new", base+1);
> +	len += snprintf(path + len, size - len, ".%s.new", base);
> +
>   	if (error_check(len, size)) {
>   		free(path);
>   		return NULL;
>   	}
> -
>   	return path;
>   }
>   
> 
> base-commit: eb5abb5c60ab60f3c2f22518d513ed187f39bd9b