[Home]

Summary:ASTERISK-22861: [patch]Specifying a null time as parameter to GotoIfTime or ExecIfTime causes segmentation fault
Reporter:Sebastian Murray-Roberts (sebmurray)Labels:
Date Opened:2013-11-18 07:42:39.000-0600Date Closed:2014-01-22 16:22:01.000-0600
Priority:MajorRegression?
Status:Closed/CompleteComponents:PBX/General
Versions:11.5.1 Frequency of
Occurrence		Constant
Related
Issues:
Environment:Red Hat Enterprise Linux Server release 6.4 (Santiago) Kernel: 2.6.32-358.0.1.el6.x86_64 HP DL 360 Server as well as VMWare virtual machineAttachments:( 0) ast_build_timing-initialize-timezone.patch
( 1) asterisk-11.5.1-TimeCheck.patch
( 2) asterisk-11.5.1-TimeCheck.patch
Description:I picked up this issue on one of our production servers, where we use GotoIfTime and ExecIfTime to control the opening and closing times of our call centres. The following lines of dialplan will cause Asterisk to seg-fault:
{noformat}
exten _X.,n,GotoIfTime(${DB(FOO/BAR)}?anylabel)
OR
exten _X.,n,ExecIfTime(${DB(FOO/BAR)}?anyapplication())
{noformat}

Where the AstDB entry FOO/BAR does not exist. We created these entries to allow for dynamically adjusting the times and setting certain parameters.

What happens is that the *data parameter to pbx_builtin_execiftime and pbx_builtin_execiftim is passed as "?anylabel" or "?anyapplication()" this passes the initial test of whether *data is zero-length or not.

In GotoIfTime, no errors regarding the format of the time are thrown, however the problem happens in the following lines:

{code}
if (ast_build_timing(&timing, s) && ast_check_timing2(&timing, tv)) {
               branch = branch1;
       } else {
               branch = branch2;
       }
       ast_destroy_timing(&timing);
{code}

ast_build_timing fails, as "s" is zero-length, this happens however at the top of the function before any other logic occurs. This means that the timing struct is never actually created and no memory is allocated to it. When ast_destroy_timing is called, it attempts to free timing->timezone which then causes a segmentation fault as timing has not explicitly been set to null, so it seems to pass the null check inside ast_destroy_timing. It then seems to be freeing a garbage pointer which causes the segmentation fault.

The same issue happens inside pbx_builtin_execiftime in the following lines:

{code}
if (!ast_build_timing(&timing, s)) {
               ast_log(LOG_WARNING, "Invalid Time Spec: %s\nCorrect usage: %s\n", s, usage);
               ast_destroy_timing(&timing);
               return -1;
       }
{code}

The only place ast_build_timing can return 0 is before the timing struct is actually initialized or created, so again when it tries to call ast_destroy_timing it attempts to free a garbage pointer and seg-faults.

I have put a solution in place in both function currently which simply checks whether ast_build_timing returns non-zero and only if it does will it attempt to call ast_destroy_timing on the timing struct.

This is probably a rather obscure bug, but thought it was worth bringing to your attention
Comments:By: Sebastian Murray-Roberts (sebmurray) 2013-11-18 07:47:37.254-0600

Attached this patch for illustrative purposes
By: Sebastian Murray-Roberts (sebmurray) 2013-11-18 07:48:24.929-0600

I have attached a patch for illustrative purposes showing what I did to fix it. I can confirm that this does fix the seg faults in both GotoIfTime and ExecIfTime
By: Rusty Newton (rnewton) 2013-11-21 13:16:59.077-0600

Looks like you may have attached the patch before your license went through. You'll have to re-attach the patch for it to show up. Thanks!
By: Sebastian Murray-Roberts (sebmurray) 2013-11-26 02:32:12.938-0600

Patch re-attached for illustrative purposes
By: Corey Farrell (coreyfarrell) 2014-01-11 02:39:02.099-0600

In addition to the applications you've mentioned this issue also applies to function IFTIME.  I've attached a patch that ensures timezone is always initialized by ast_build_timing.  This issue/patch applies to all supported branches + trunk.
By: Corey Farrell (coreyfarrell) 2014-01-11 02:49:54.458-0600

Looks like I spoke too soon, function IFTIME bails early if timespec is blank.  I still think ast_destroy_timing should be safe after a call to ast_build_timing.