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?
--
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
