libtraceevent: Reset right arg when copying TEP_PRINT_OP

Commit Message

Namhyung Kim June 6, 2022, 7:58 p.m. UTC

When processing a TEP_PRINT_OP type arg, the original arg was copied
to the left arg and resets itself.  But it misses the reset the right
in some places and it could result in a use-after-free.

A fuzzer test found out that something like below can trigger it

  print fmt: "", c * ((3 * t)[

At the time it sees the "[" token, the arg would have like

  arg->type = TEP_PRINT_OP
  arg->op.op = "*"
  arg->op.left = (arg of 3)
  arg->op.right = (arg of t)

and it creates a new left and copies the contents.  Also it resets
itself with

  arg->op.op = "["
  arg->op.left = (new left)

But it can have the same arg->op.right if the process_array() fails
before setting it.  It should reset the right pointer as it passed the
ownership before.  The same thing can happend for process_cond().

Signed-off-by: Namhyung Kim <[e-mail]>
---
 src/event-parse.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Steven Rostedt June 8, 2022, 3:17 p.m. UTC | #1

On Mon,  6 Jun 2022 12:58:59 -0700
Namhyung Kim <[e-mail]> wrote:

> When processing a TEP_PRINT_OP type arg, the original arg was copied
> to the left arg and resets itself.  But it misses the reset the right
> in some places and it could result in a use-after-free.
> 
> A fuzzer test found out that something like below can trigger it
> 
>   print fmt: "", c * ((3 * t)[
> 
> At the time it sees the "[" token, the arg would have like
> 
>   arg->type = TEP_PRINT_OP
>   arg->op.op = "*"
>   arg->op.left = (arg of 3)
>   arg->op.right = (arg of t)
> 
> and it creates a new left and copies the contents.  Also it resets
> itself with
> 
>   arg->op.op = "["
>   arg->op.left = (new left)
> 
> But it can have the same arg->op.right if the process_array() fails
> before setting it.  It should reset the right pointer as it passed the
> ownership before.  The same thing can happend for process_cond().
> 
> Signed-off-by: Namhyung Kim <[e-mail]>
> ---
>  src/event-parse.c | 2 ++
>  1 file changed, 2 insertions(+)

Applied. Thanks Namhyung!

-- Steve

Patch

diff --git a/src/event-parse.c b/src/event-parse.c
index 8b839cb..8f4fb59 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -2317,6 +2317,7 @@  process_op(struct tep_event *event, struct tep_print_arg *arg, char **tok)
 		arg->type = TEP_PRINT_OP;
 		arg->op.op = token;
 		arg->op.left = left;
+		arg->op.right = NULL;
 		arg->op.prio = 0;
 
 		/* it will set arg->op.right */
@@ -2422,6 +2423,7 @@  process_op(struct tep_event *event, struct tep_print_arg *arg, char **tok)
 		arg->type = TEP_PRINT_OP;
 		arg->op.op = token;
 		arg->op.left = left;
+		arg->op.right = NULL;
 
 		arg->op.prio = 0;