parser: fix Normalizatin infinite loop

Expression simplification can get into an infinite loop due to eps
pairs hiding behind and alternation that can't be caught by
normalize_eps() (which exists in the first place to stop a similar
loop).

The loop in question happens in AltNode::normalize when a subtree has
the following structure.

1. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too

                   alt
                   /\
                  /  \
                 /    \
               eps    alt
                      /\
                     /  \
                    /    \
                  alt    eps
                  /\
                 /  \
                /    \
               eps   eps

2. if (normalize_eps(dir)) results in

                   alt
                   /\
                  /  \
                 /    \
               alt    eps
               /\
              /  \
             /    \
           alt    eps
           /\
          /  \
         /    \
       eps   eps

3. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too

                   alt
                   /\
                  /  \
                 /    \
               alt    alt
              /\        /\
             /  \      /  \
            /    \    /    \
          eps    eps eps   eps

4. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too

                   alt
                   /\
                  /  \
                 /    \
               eps    alt
                      /\
                     /  \
                    /    \
                  eps    alt
                         /\
                        /  \
                       /    \
                     eps   eps

5. if (normalize_eps(dir)) results in

                  alt
                   /\
                  /  \
                 /    \
                alt   eps
                /\
               /  \
              /    \
            eps    alt
                    /\
                   /  \
                  /    \
                 eps   eps

6. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too

                  alt
                   /\
                  /  \
                 /    \
                eps   alt
                       /\
                      /  \
                     /    \
                    alt  eps
                    /\
                   /  \
                  /    \
                eps   eps

back to beginning of cycle

Fix this by detecting the creation of an eps_pair in rotate_node(),
that pair can be immediately eliminated by simplifying the tree in that
step.

In the above cycle the pair creation is caught at step 3 resulting
in

3. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too

                   alt
                   /\
                  /  \
                 /    \
               alt    eps
               /\
              /  \
             /    \
           eps    eps

4.  elseif (child[dir]->is_type(ALT_NODE)) rotate_node too

                   alt
                   /\
                  /  \
                 /    \
               eps   alt
                      /\
                     /  \
                    /    \
                  eps    eps

whch gets reduces to

                   alt
                   /\
                  /  \
                 /    \
               eps   eps

breaking the normalization loop. The degenerate alt node will be caught
in turn when its parent is dealt with.

This needs to be backported to all releases

Closes: https://gitlab.com/apparmor/apparmor/-/issues/398
Fixes: 846cee506 ("Split out parsing and expression trees from regexp.y")
Reported-by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: John Johansen <john.johansen@canonical.com>
This commit is contained in:
John Johansen 2024-06-05 18:38:29 -07:00
parent 70ddb0ca5b
commit b6d9d9d8b6
2 changed files with 42 additions and 8 deletions

View file

@ -189,6 +189,19 @@ void Node::dump_syntax_tree(ostream &os)
* a b c T
*
*/
static Node *simplify_eps_pair(Node *t)
{
if (t->is_type(NODE_TYPE_TWOCHILD) &&
t->child[0] == &epsnode &&
t->child[1] == &epsnode) {
t->release();
return &epsnode;
}
return t;
}
static void rotate_node(Node *t, int dir)
{
// (a | b) | c -> a | (b | c)
@ -197,7 +210,9 @@ static void rotate_node(Node *t, int dir)
t->child[dir] = left->child[dir];
left->child[dir] = left->child[!dir];
left->child[!dir] = t->child[!dir];
t->child[!dir] = left;
// check that rotation didn't create (E | E)
t->child[!dir] = simplify_eps_pair(left);
}
/* return False if no work done */
@ -209,13 +224,7 @@ int TwoChildNode::normalize_eps(int dir)
// Ea -> aE
// Test for E | (E | E) and E . (E . E) which will
// result in an infinite loop
Node *c = child[!dir];
if (c->is_type(NODE_TYPE_TWOCHILD) &&
&epsnode == c->child[dir] &&
&epsnode == c->child[!dir]) {
c->release();
c = &epsnode;
}
Node *c = simplify_eps_pair(child[!dir]);
child[!dir] = child[dir];
child[dir] = c;
return 1;

View file

@ -0,0 +1,25 @@
#
#=Description caused an infinite loop in expr normalization
#=EXRESULT PASS
# This test triggers an infinite loop bug in expr normalization
# Note: this test might be able to be reduced more but, each element appears
# to be required to trigger the bug.
# that is the initial var assignment, += with the "comment" at the end
# (which is a separate bug), the expansion in the 2nd variable and then
# the use of the 2nd variable.
# This seems to be due to difference in consistency check between expansion
# at parse time and variable expansion.
# eg. expanding @{exec_path} manually will result in a failure to parse
# see: https://gitlab.com/apparmor/apparmor/-/issues/398
@{var}=*-linux-gnu*
@{var}+=*-suse-linux* #aa:only opensuse
@{exec_path} = /{,@{var}/}t
profile test {
@{exec_path} mr,
}