[SR-Users] [PATCH] Memory corruption using s.substr transformation

[Martin Mikkelsen]

Hello,

I have a kamailio 3.3.4 server running on x86-64 Linux, and I have a
script which looks something like this:

  $var(gwruri) = $rU;
  if ($(var(gwruri){s.substr,0,1}) == "+") {
    $var(gwruri) = $(var(gwruri){s.substr,1,0});
  }

When this script is run with say $rU = "+0000009724" (real number
removed).  $var(gwruri) should contain the same but without the +, this
is not the case, $var(gwruri) ends up being "0000007724".

This smells memory corruption. When run under valgrind, valgrind
generates this error when that code is run:

==1206== Source and destination overlap in strncpy(0x55e3b2a, 0x55e3b2b, 10)
==1206==    at 0x4C25ACF: strncpy (mc_replace_strmem.c:339)
==1206==    by 0x2A28BB92: set_var_value (pv_svar.c:122)
==1206==    by 0x2A2800D1: pv_set_scriptvar (pv_core.c:1683)
==1206==    by 0x45E8F8: lval_assign (lvalue.c:353)
==1206==    by 0x416A78: do_action (action.c:1524)
==1206==    by 0x41E465: run_actions (action.c:1644)
==1206==    by 0x4177FD: do_action (action.c:1136)
==1206==    by 0x41E465: run_actions (action.c:1644)
==1206==    by 0x4177FD: do_action (action.c:1136)
==1206==    by 0x41E465: run_actions (action.c:1644)
==1206==    by 0x419C67: do_action (action.c:1140)
==1206==    by 0x41E465: run_actions (action.c:1644)
==1206==

I am not very familiar with the kamailio source code, but as far as I
can tell this happens because the strncpy() in set_var_value() in
pv_svar.c copies the string directly within the same overlapping memory
area. The same memory area is used because the tr_eval_string() function
and the TR_S_SUBSTR-code in pv_trans.c reuses the input buffer and just
increments the pointer and since the source pvar is the same as the
destination pvar the strncpy() ends up copying between overlapping
memory area.

I am not sure what the best fix would be for that, but I have attached a
patch which copies the string in the TR_S_SUBST code to _tr_buffer and
returns that buffer instead like a lot of the other transformations in
that function does.

If this fix is correct some other transformation functions probably
needs to be corrected as well.

-- 
Martin Mikkelsen, Zisson AS