Patchwork [v3,libnetfilter_acct,3/29] bugfix: correct xml name parsing

login
register
mail settings
Submitter Michael Zintakis
Date July 10, 2013, 6:25 p.m.
Message ID <1373480727-11254-4-git-send-email-michael.zintakis@googlemail.com>
Download mbox | patch
Permalink /patch/258191/
State Changes Requested
Headers show

Comments

Michael Zintakis - July 10, 2013, 6:25 p.m.
* allow accounting object names to be properly encoded and displayed when xml
output is needed, to fully conform to the xml specification.

Signed-off-by: Michael Zintakis <michael.zintakis@googlemail.com>
---
 src/libnetfilter_acct.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)
Pablo Neira - July 15, 2013, 10:24 p.m.
On Wed, Jul 10, 2013 at 07:25:01PM +0100, Michael Zintakis wrote:
> * allow accounting object names to be properly encoded and displayed when xml
> output is needed, to fully conform to the xml specification.
> 
> Signed-off-by: Michael Zintakis <michael.zintakis@googlemail.com>
> ---
>  src/libnetfilter_acct.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libnetfilter_acct.c b/src/libnetfilter_acct.c
> index ba89e2d..4d87da3 100644
> --- a/src/libnetfilter_acct.c
> +++ b/src/libnetfilter_acct.c
> @@ -228,6 +228,43 @@ uint64_t nfacct_attr_get_u64(struct nfacct *nfacct, enum nfacct_attr_type type)
>  }
>  EXPORT_SYMBOL(nfacct_attr_get_u64);
>  
> +static
> +void parse_nfacct_name_xml(char *buf, const char *name)

Please, use the prefix nfacct_* for consistency. My suggestion for
this function name is nfacct_name_to_xml.

> +{
> +	static const char escape_chars[] = "\"'<>&";

You have to escape the apostroph as well.

> +	int length;
> +	int n;
> +	char e[10];
> +	const char *p;
> +
> +	if (buf == NULL)
> +		return;

This checking is superfluous, it always evaluates false.

> +
> +	buf[0] = '\0';

Don't need this initial zeroing.

> +	if (name == NULL)
> +		return;

Same thing above.

> +	length = strcspn(name, escape_chars);
> +	if (length > 0 && name[length] == 0) {
> +		/* no escaping required */
> +		strncat(buf, name, length);
> +	} else {
> +		for (p = strpbrk(name, escape_chars); p != NULL;
> +		     p = strpbrk(name, escape_chars)) {
> +			if (p > name)
> +				strncat(buf, name, p - name);
> +
> +			n = *p;
> +			snprintf(e, sizeof(e), "&#%d;", n);
> +			strncat(buf, e, strlen(e));

Please use snprintf to build the string (instead of strncat) you can
use BUFFER_SIZE to update the buffer offset.

> +			name = p + 1;
> +		}
> +
> +		/* strncat the rest */
> +		strncat(buf, name, length);
> +	}
> +}
> +
>  static int
>  nfacct_snprintf_plain(char *buf, size_t rem, struct nfacct *nfacct,
>  		      uint16_t flags)
> @@ -292,12 +329,16 @@ nfacct_snprintf_xml(char *buf, size_t rem, struct nfacct *nfacct,
>  {
>  	int ret = 0;
>  	unsigned int size = 0, offset = 0;
> +	char nfacct_name[NFACCT_NAME_MAX * 6 + 1];

Why this buffer length?

> +	parse_nfacct_name_xml(nfacct_name,
> +				nfacct_attr_get_str(nfacct,
> +						    NFACCT_ATTR_NAME));
>  	ret = snprintf(buf, rem,
>  			"<obj><name>%s</name>"
>  			"<pkts>%.20"PRIu64"</pkts>"
>  			"<bytes>%.20"PRIu64"</bytes>",
> -			nfacct_attr_get_str(nfacct, NFACCT_ATTR_NAME),
> +			nfacct_name,
>  			nfacct_attr_get_u64(nfacct, NFACCT_ATTR_BYTES),
>  			nfacct_attr_get_u64(nfacct, NFACCT_ATTR_PKTS));
>  	BUFFER_SIZE(ret, size, rem, offset);
> -- 
> 1.8.3.1
> 
--
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

Patch

diff --git a/src/libnetfilter_acct.c b/src/libnetfilter_acct.c
index ba89e2d..4d87da3 100644
--- a/src/libnetfilter_acct.c
+++ b/src/libnetfilter_acct.c
@@ -228,6 +228,43 @@  uint64_t nfacct_attr_get_u64(struct nfacct *nfacct, enum nfacct_attr_type type)
 }
 EXPORT_SYMBOL(nfacct_attr_get_u64);
 
+static
+void parse_nfacct_name_xml(char *buf, const char *name)
+{
+	static const char escape_chars[] = "\"'<>&";
+	int length;
+	int n;
+	char e[10];
+	const char *p;
+
+	if (buf == NULL)
+		return;
+
+	buf[0] = '\0';
+	if (name == NULL)
+		return;
+
+	length = strcspn(name, escape_chars);
+	if (length > 0 && name[length] == 0) {
+		/* no escaping required */
+		strncat(buf, name, length);
+	} else {
+		for (p = strpbrk(name, escape_chars); p != NULL;
+		     p = strpbrk(name, escape_chars)) {
+			if (p > name)
+				strncat(buf, name, p - name);
+
+			n = *p;
+			snprintf(e, sizeof(e), "&#%d;", n);
+			strncat(buf, e, strlen(e));
+			name = p + 1;
+		}
+
+		/* strncat the rest */
+		strncat(buf, name, length);
+	}
+}
+
 static int
 nfacct_snprintf_plain(char *buf, size_t rem, struct nfacct *nfacct,
 		      uint16_t flags)
@@ -292,12 +329,16 @@  nfacct_snprintf_xml(char *buf, size_t rem, struct nfacct *nfacct,
 {
 	int ret = 0;
 	unsigned int size = 0, offset = 0;
+	char nfacct_name[NFACCT_NAME_MAX * 6 + 1];
 
+	parse_nfacct_name_xml(nfacct_name,
+				nfacct_attr_get_str(nfacct,
+						    NFACCT_ATTR_NAME));
 	ret = snprintf(buf, rem,
 			"<obj><name>%s</name>"
 			"<pkts>%.20"PRIu64"</pkts>"
 			"<bytes>%.20"PRIu64"</bytes>",
-			nfacct_attr_get_str(nfacct, NFACCT_ATTR_NAME),
+			nfacct_name,
 			nfacct_attr_get_u64(nfacct, NFACCT_ATTR_BYTES),
 			nfacct_attr_get_u64(nfacct, NFACCT_ATTR_PKTS));
 	BUFFER_SIZE(ret, size, rem, offset);