Summary:ASTERISK-23391: Audit dialplan function usage of channel variable
Reporter:Corey Farrell (coreyfarrell)Labels:
Date Opened:2014-02-27 14:41:01.000-0600Date Closed:2014-03-27 14:28:33
Versions:SVN 11.7.0 12.0.0 Frequency of
causesASTERISK-23559 app_voicemail fails to load after fix to dialplan functions
is related toASTERISK-22982 CEL/cel_custom: Using non-standard func_channel CHANNEL(..) variables causes segfaults
Environment:Attachments:( 0) functions-check-chan-1.8.patch
( 1) functions-check-chan-11.patch
( 2) functions-check-chan-12.patch
Description:Dialplan functions can be called from AMI without a channel.  This allows some functions to be executed in the global context.  Some functions do not check for NULL channels and can crash when executed as a global function.
Comments:By: Corey Farrell (coreyfarrell) 2014-02-28 10:37:26.408-0600

Attached are patches for each branch.  I audited 1.8 using:
bq. grep -R * -e 'static struct ast_custom_function '
This gave me the a list of all custom functions.  When merging to each new version I compared the list, and only checked new functions.  This means if a function that existed in 1.8 or 11 started using chan in the variable declaration area of 12, I might not have noticed.  For trunk I did not do an audit, I copied my changes from 12 and compiled.

I do not understand func_groupcount.c and can't tell if chan==NULL is safe.  Also I don't have deps for apps/app_jack.c, so I could not compile it.  I visually inspected the result on each version of asterisk and it looks good.  All other files that I changed were successfully compiled in all versions of asterisk.  No runtime testing has been done.

These patches have been provided separate for each version as I had many merge issues.  In one file the change was applied to the wrong function.  I'm less concerned with the changes I've made, more concerned with verifying they were applied correctly to higher versions.

By: Corey Farrell (coreyfarrell) 2014-03-03 14:00:26.663-0600

func_groupcount patch is for all branches.  RB postings coming later this week once I've had time to recheck everything.

By: Walter Doekes (wdoekes) 2014-03-25 05:47:37.019-0500

Linking ASTERISK-22982 because it was caused by the lack of check in static int func_channel_read. That one is fixed by the posted review.

However, ASTERISK-22982 would probably be better served by removing the use of ${CHANNEL()} altogether.

By: abelbeck (abelbeck) 2014-03-29 13:33:20.173-0500

There is a regression in /branches/11/apps/app_voicemail.c with Revision 411314

defaultlanguage is undefined, possibly you wanted to use DEFAULT_LANGUAGE instead.
WARNING[1885]: loader.c:439 in load_dynamic_module: Error loading module 'app_voicemail.so': /usr/lib/asterisk/modules/app_voicemail.so: undefined symbol: defaultlanguage

By: Corey Farrell (coreyfarrell) 2014-03-29 13:36:36.943-0500

I have opened a new ticket for the regression ASTERISK-23559, a patch is posted to reviewboard.

defaultlanguage is the correct variable to use.  DEFAULT_LANGUAGE is always "en", defaultlanguage is read from asterisk.conf.