12 Jul
2016
12 Jul
'16
8:59 a.m.
Hello,
indeed, the pv names cache doesn't invalidate items at runtime, relying
on the old model of static name string that evaluates its variables to
get the dynamic name value.
I would be fine with solution to overcome this:
1) have some threshold of items/memory after which to start removing
items. Ideally, this should be done only with the variables that have
dynamic names (e,g for $sht(...), but not for $ru) -- iirc, there are
some flags or type field in declaration of the pv that can be extended
to cover this case.
2) have a core parameter (might be used only by the app_lua and the
other embedded languages modules) to control whether to cache or not pv
name specs for vars with dynamic name
One thig to double check is whether we need or not a free function for
pv name spec, might be that some particular vars allocate memory some
just point to other values...
Somehow it is pretty much obvious there would be security implications
when taking values from incoming traffic without sanity/safety checks.
Cheers,
Daniel
On 12/07/16 14:19, Timo Teras wrote:
Hi,
I was recently debugging a memory leak in Kamailio running lua code.
And it turned out to be rather nasty interaction with the PV cache and
the Lua code. Technically, this is bug in Kamailio. However, we ended
up fixing the lua code because the same bug also introduces security
issues.
The problem is that PV cache never releases cached items. Normally this
is not a problem since in .cfg files you can only access PVs with fixed
name. In cases where dynamic part is needed, like $sht() access based
on variable, the expansion is done in the PV expression evaluation and
the actual PV name is still static.
However, app_lua exposes sr.pv.* where the PV name is a string. The lua
application is free to construct it as it wishes. The case I had was
using lua code to expand a variable, and then do sr.pv.get/sets on the
expanded "$sht(table=>"..key..")" type expression.
This resulted PV core leaking cached entry for each unique key. So I
wonder if should limit the PV cache size, and e.g. make the bucket
chain size limited (start releasing oldest cache entries when the
bucket becomes full)? Or if this is not possible, e.g. due to memory
allocation semantics, at least put big fat warning to the app_lua's
sr.pv.* documentation on this.
I ended up just fixing the application to set it first to "$var(tmp)"
and then use that in the PV query. The security implication is
noticeable if 'key' comes from untrusted source (e.g. SIP message) as
then an attacker can inject string for PV expansion...
Thanks,
Timo
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
--
Daniel-Constantin Mierla
http://www.asipto.com - http://www.kamailio.org
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda