[asterisk-bugs] [JIRA] (ASTERISK-22982) CEL/cel_custom: Using non-standard func_channel CHANNEL(..) variables causes segfaults

[Walter Doekes (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-22982?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=212909#comment-212909 ] 

Walter Doekes commented on ASTERISK-22982:
------------------------------------------

Summarizing:

1. The dummy channel exists so we can do text substitution on it, like CSV_QUOTE().

2. There doesn't seem to be a reason to prefer $\{CHANNEL(accountcode)} over $\{accountcode}.

3. Fixing this would require: (a) adding the appropriate vars and (b) deprecating the CHANNEL/CALLERID function calls.

4. After that you could still get things to crash by calling functions that manipulate other channel data than just variables, but you wouldn't be inclined to, since there are no recent examples that do so. (Perhaps when registering a function the registerer should specify whether it is safe to call it on a dummy channel: string functions get a yes, all functions that touch more than just variables get a no.)
                
> CEL/cel_custom: Using non-standard func_channel CHANNEL(..) variables causes segfaults
> --------------------------------------------------------------------------------------
>
>                 Key: ASTERISK-22982
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-22982
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: CEL/cel_custom, CEL/cel_sqlite3_custom
>    Affects Versions: 11.7.0
>            Reporter: Walter Doekes
>            Severity: Minor
>
> I'm confused here.
> In {{cel_odbc}} (and {{cel_pgsql}}) I simply get a certain set of columns, like {{cid_name}}, {{accountcode}}, and so on.
> In {{cel_custom}} (and {{cel_sqlite}}) I have to specify channel functions, like {{$\{CALLERID(name)}}} and {{$\{CHANNEL(accountcode)}}}.
> Those will get run through ast_str_substitute_variables_full.
> -But, then the channel isn't completely initialized -- which it apparently isn't -- we can't get all possible values out of that:-
> But, since the channel is a dummy channel, we can't get all possible values out of that:
> For instance, using {{$\{CHANNEL(channeltype)}}} yields this:
> {noformat}
> Program received signal SIGSEGV, Segmentation fault.
> 445			locked_copy_string(chan, buf, ast_channel_tech(chan)->type, len);
> (gdb) print ast_channel_tech(chan)
> $1 = (const struct ast_channel_tech *) 0x0
> (gdb) back
> #0  0x00007fffc73dc270 in func_channel_read (chan=0x7ffff0000b28, function=0x7ffff7fc7a00 "CHANNEL", data=0x7ffff7fc7a08 "channeltype", buf=0x7ffff00034b8 "", len=4096) at func_channel.c:445
> #1  0x0000000000532c0d in ast_func_read2 (chan=0x7ffff0000b28, function=0x7ffff0003478 "CHANNEL(channeltype)", str=0x7ffff7fc7b38, maxlen=0) at pbx.c:4127
> #2  0x00000000005331f1 in ast_str_substitute_variables_full (buf=0x7ffff7fc7c20, maxlen=0, c=0x7ffff0000b28, headp=0x0, templ=0x7ffff00021e3 "{UNIQUETREE}/${CHANNEL(channeltype)})", used=0x7ffff7fc7c30)
>     at pbx.c:4256
> #3  0x0000000000533170 in ast_str_substitute_variables_full (buf=0x7ffff7fc7d00, maxlen=0, c=0x7ffff0000b28, headp=0x0, 
>     templ=0xe9c85e "ype})},${CSV_QUOTE(${eventtime})},${CSV_QUOTE(${CALLERID(name)})},${CSV_QUOTE(${CALLERID(num)})},${CSV_QUOTE(${CALLERID(ANI)})},${CSV_QUOTE(${CALLERID(RDNIS)})},${CSV_QUOTE(${CALLERID(DNID)})},${CSV_Q"..., used=0x7ffff7fc7cc8) at pbx.c:4246
> #4  0x000000000053373a in ast_str_substitute_variables (buf=0x7ffff7fc7d00, maxlen=0, chan=0x7ffff0000b28, 
>     templ=0xe9c84a "${CSV_QUOTE(${eventtype})},${CSV_QUOTE(${eventtime})},${CSV_QUOTE(${CALLERID(name)})},${CSV_QUOTE(${CALLERID(num)})},${CSV_QUOTE(${CALLERID(ANI)})},${CSV_QUOTE(${CALLERID(RDNIS)})},${CSV_QUOTE(${CALLE"...) at pbx.c:4343
> #5  0x00007fffc77ec7e3 in custom_log (event=0x7fffc0032fb0, userdata=0x0) at cel_custom.c:149
> #6  0x00000000004c8872 in handle_event (data=0x7fffc0033138) at event.c:1520
> {noformat}
> Dereferencing 0x0 for 'type' is obviously bad. Same goes for {{ast_channel_zone(chan)}} and probably a bunch of others too.
> _The question_
> Did we really want to call these functions? Or do we only want the {{$\{CSV_QUOTE()}}} expansion?
> If the former is the case, why don't I have access to my other channel variables? If the latter, why the detour through the CHANNEL and CALLERID functions. Couldn't we call the variables {{$\{cid_name}}} and {{$\{accountcode}}} instead?
> _Answering parts of my question_
> Apparently we get a bogus channel that doesn't hold any more info than exactly the variables put in from the CEL event.
> {noformat}
> dummy = ast_cel_fabricate_channel_from_event(event);
> {noformat}
> Like I mentioned in the first line: this sets the wrong expectations and causes confusion.
> And it seems to me that calling the {{CALLERID}} and friends would cause more overhead than loading up the vars through a bunch of {{ast_var_assign}}'s.
> Is there a reason why we didn't do that in the first place?
> Cheers,
> Walter

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.asterisk.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira