tree 060aa8fcc55424e45759bc6df83cdf45178ef5e7
parent 47a08e84506145512967f765118dc1e27dcf6ea4
author Aurelien DARRAGON <adarragon@haproxy.com> 1725886759 +0200
committer Willy Tarreau <w@1wt.eu> 1725954952 +0200
encoding latin1

BUG/MEDIUM: pattern: prevent UAF on reused pattern expr

Since c5959fd ("MEDIUM: pattern: merge same pattern"), UAF (leading to
crash) can be experienced if the same pattern file (and match method) is
used in two default sections and the first one is not referenced later in
the config. In this case, the first default section will be cleaned up.
However, due to an unhandled case in the above optimization, the original
expr which the second default section relies on is mistakenly freed.

This issue was discovered while trying to reproduce GH #2708. The issue
was particularly tricky to reproduce given the config and sequence
required to make the UAF happen. Hopefully, Github user @asmnek not only
provided useful informations, but since he was able to consistently
trigger the crash in his environment he was able to nail down the crash to
the use of pattern file involved with 2 named default sections. Big thanks
to him.

To fix the issue, let's push the logic from c5959fd a bit further. Instead
of relying on "do_free" variable to know if the expression should be freed
or not (which proved to be insufficient in our case), let's switch to a
simple refcounting logic. This way, no matter who owns the expression, the
last one attempting to free it will be responsible for freeing it.
Refcount is implemented using a 32bit value which fills a previous 4 bytes
structure gap:

        int                        mflags;               /*    80     4 */

        /* XXX 4 bytes hole, try to pack */

        long unsigned int          lock;                 /*    88     8 */
(output from pahole)

Even though it was not reproduced in 2.6 or below by @asmnek (the bug was
revealed thanks to another bugfix), this issue theorically affects all
stable versions (up to c5959fd), thus it should be backported to all
stable versions.

(cherry picked from commit 68cfb222b559b909415c9aa92114ca42c9a93459)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit fc0af6f2bb0bf99d9c84e888e3daaa5529da2a5c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit fccf836bed93c0976012e940cfefeea64e352df5)
Signed-off-by: Willy Tarreau <w@1wt.eu>