Re: [libnftnl PATCH] mxml: test if the root node name is initialized before to compare it.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



El 04/03/15 a las 20:20, Pablo Neira Ayuso escribió:
On Wed, Mar 04, 2015 at 08:03:18PM +0100, Pablo Neira Ayuso wrote:
On Tue, Mar 03, 2015 at 02:45:34PM +0100, Álvaro Neira Ayuso wrote:
El 03/03/15 a las 00:35, Pablo Neira Ayuso escribió:
On Mon, Mar 02, 2015 at 08:58:38PM +0100, Alvaro Neira Ayuso wrote:
If the root node name is not correctly initialized, we have a crash.

Under what circunstances may tree->value.opaque be NULL?

The function mxmlLoadFile doesn't work correctly. With my patch to
import ruleset in json and xml. I have seen that if we use a ruleset
in json and use the command nft import xml.

OK, so this bug can be triggered by malformed input.

The function mxmlLoadFile returns a tree without any information
inside.  Therefore, when we try to verify that the name of the first
node is the same of the parameter, we have a crash.

We have two ways to solve it. Test if the node is NULL and:
  1- show an error that the node is not found.
  2- show an error that the input is not correctly.

I have thought that the best is the first. But I can change it.

I think we have to make robust checks on the input, if what we get is
not "sane" then it's not worth going further.

So I'd suggest you modify this lines a bit below:

         if (tree->value.opaque == NULL) {
                 err->error = NFT_PARSE_EBADINPUT;
                 goto err;
         }

to look like:

         if (tree == NULL || tree->value.opaque == NULL) {
                 err->error = NFT_PARSE_EBADINPUT;
                 goto err;
         }

---
  src/mxml.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mxml.c b/src/mxml.c
index 0001ba0..b68f86f 100644
--- a/src/mxml.c
+++ b/src/mxml.c
@@ -43,7 +43,8 @@ mxml_node_t *nft_mxml_build_tree(const void *data, const char *treename,
  		goto err;
  	}

-	if (strcmp(tree->value.opaque, treename) == 0)
+	if (tree->value.opaque != NULL &&
+	    strcmp(tree->value.opaque, treename) == 0)
  		return tree;

  	err->error = NFT_PARSE_EMISSINGNODE;

Actually your patch looks correct because you're returning "missing
node" which is triggering the crash, right?

Yes, the first node of the tree is missing and we have a crash.

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




[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux