[libnftnl PATCH] common: events: be more strict when placing the \0 character

[libnftnl PATCH] common: events: be more strict when placing the \0 character

[Arturo Borrero Gonzalez]

Previous to this patch, the code path can potentially print an empty
buffer with the \0 at the end of the buffer.

Be more strict and place the \0 character in the first position if the
buffer is empty.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 src/common.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/common.c b/src/common.c
index 1b600f1..c81a1d8 100644
--- a/src/common.c
+++ b/src/common.c
@@ -114,9 +114,10 @@ int nft_event_header_snprintf(char *buf, size_t size, uint32_t type,
 int nft_event_header_fprintf(FILE *fp, uint32_t type, uint32_t flags)
 {
 	char buf[64]; /* enough for the maximum string length above */
+	int ret;
 
-	nft_event_header_snprintf(buf, sizeof(buf), type, flags);
-	buf[sizeof(buf) - 1] = '\0';
+	ret = nft_event_header_snprintf(buf, sizeof(buf), type, flags);
+	buf[ret] = '\0';
 
 	return fprintf(fp, "%s", buf);
 }
@@ -140,9 +141,10 @@ int nft_event_footer_snprintf(char *buf, size_t size, uint32_t type,
 int nft_event_footer_fprintf(FILE *fp, uint32_t type, uint32_t flags)
 {
 	char buf[32]; /* enough for the maximum string length above */
+	int ret;
 
-	nft_event_footer_snprintf(buf, sizeof(buf), type, flags);
-	buf[sizeof(buf) - 1] = '\0';
+	ret = nft_event_footer_snprintf(buf, sizeof(buf), type, flags);
+	buf[ret] = '\0';
 
 	return fprintf(fp, "%s", buf);
 }

Re: [libnftnl PATCH] common: events: be more strict when placing the \0 character

[Pablo Neira Ayuso]

On Mon, Jul 14, 2014 at 02:43:35PM +0200, Arturo Borrero Gonzalez wrote:
> Previous to this patch, the code path can potentially print an empty
> buffer with the \0 at the end of the buffer.
> 
> Be more strict and place the \0 character in the first position if the
> buffer is empty.
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  src/common.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/src/common.c b/src/common.c
> index 1b600f1..c81a1d8 100644
> --- a/src/common.c
> +++ b/src/common.c
> @@ -114,9 +114,10 @@ int nft_event_header_snprintf(char *buf, size_t size, uint32_t type,
>  int nft_event_header_fprintf(FILE *fp, uint32_t type, uint32_t flags)
>  {
>  	char buf[64]; /* enough for the maximum string length above */
> +	int ret;
>  
> -	nft_event_header_snprintf(buf, sizeof(buf), type, flags);
> -	buf[sizeof(buf) - 1] = '\0';
> +	ret = nft_event_header_snprintf(buf, sizeof(buf), type, flags);
> +	buf[ret] = '\0';

ret can be 64 at worst case, then you have ret[64] would be an
off-by-one memory access (out of bound).

BTW, could you review all_snprintf functions? We should retain the
snprintf semantics, ie. always nul-terminate strings if

        offset < buffer_size

Otherwise, return the string without the nul-termination.

Re: [libnftnl PATCH] common: events: be more strict when placing the \0 character

[Arturo Borrero Gonzalez]

On 15 July 2014 18:42, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> BTW, could you review all_snprintf functions? We should retain the
> snprintf semantics, ie. always nul-terminate strings if
>
>         offset < buffer_size
>
> Otherwise, return the string without the nul-termination.

Sure, thanks.

-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html