Summary:	ASTERISK-09045: AEL security risk in switch blocks
Reporter:	pkempgen (pkempgen)	Labels:
Date Opened:	2007-03-19 10:36:56	Date Closed:	2007-03-20 14:50:26
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	PBX/pbx_ael
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	The AEL compiler generates extensions from the "case"s in a switch{} block. A SIP user might guess one of the sw-X-.. extensions and execute dialplan code which he shouldn't be allowed to execute. See the mail in additional information from the -dev list. Steve Murphy suggested to open a bug report. ****** ADDITIONAL INFORMATION ****** On Mon, 2007-03-19 at 14:39 +0100, Philipp Kempgen wrote: > > Steve Murphy wrote: > > >> > > On Mon, 2007-03-19 at 12:32 +0100, Philipp Kempgen wrote: >>> > >> Philipp Kempgen wrote: >>> > >> >>>> > >>> Sergey Okhapkin wrote: >>>> > >>> >>>>> > >>>> AEL needs to use extensions when compiling "switch" statement, asterisk >>>>> > >>>> extensions pattern match is being used for "default" case. >>>>> > >>>> >>>>> > >>>> On Monday 19 March 2007 06:39, Philipp Kempgen wrote: >>>>>> > >>>>> Philipp Kempgen wrote: >>>>>>> > >>>>>> It seems like AEL compiles labels into extensions. >>>>>>> > >>>>>> So a users could directly dial to a label which seems >>>>>>> > >>>>>> like a security risk to me. Am I missing something? >>>>>> > >>>>> Need to correct myself: AEL compiles the cases in a switch >>>>>> > >>>>> block into extensions. Labels remain untouched. But that >>>>>> > >>>>> doesn't make it any better. >>>> > >>> Features are not an excuse for weak security. :P >>> > >> And although it is implemented this way the AEL compiler could >>> > >> use something like this for the default case: >>> > >> >>> > >> exten => 123,n,GotoIf($["${switchvar}" = "BUSY"]?user_busy) >>> > >> exten => 123,n,GotoIf($["${switchvar}" = "NOANSWER"]?user_unavail) >>> > >> exten => 123,n,Goto(default) >> > > >> > > Philipp-- >> > > >> > > Please help me to understand the security implications here. I could >> > > invest some time and re-do the stuff for switch statements without using >> > > extensions... is it >> > > that the creation of the extra extensions might be addressable from >> > > outside your >> > > org? So, putting Dial() commands to targets outside the org could be the >> > > risk? Are there others that I'm not thinking of? AEL compiles switch >> > > cases into extensions with names like: sw-<a generated integer>-<Case >> > > Label>, and for the >> > > default condition, it generates "." as the case label, eg. sw-32-. >> > > >> > > So, as I see it, the risk is that a clever attacker will make >> > > sip/iax/etc calls to your system with addresses like "sw-2-BUSY", (PSTN >> > > calls would only be able to provide numeric extension names) looking for >> > > a switch case that might give him a free ticket to the PSTN? > > > > Exactly. One of the internal users might figure this out > > and change settings of other users, listen to other users' > > voicemail, whatever. Although this is not very likely I > > see the potential risk. Thanks, Philipp-- Could you open a bug on bugs.digium.com, and explain this there, and I'll schedule a re-do that avoids the creation of extensions for switches. I don't see that this is a critical risk, as DISA **should** be provided with a password, if you care about security, and Authenticate just hangs up on failure instead of returning a result you'd want to use in a switch... but you are right, in unexpected places, and in unexpected ways, some wiseguy with a sip connection could be trying random (or worse yet, thoughtful) extensions of the form sw-3-x to get potentially juicy extensions to be executed. How fruitful this would be, I cannot predict, but there is a possibility that it could be fruitful, I must admit. murf > > > > Regards, > >   Philipp > > _______________________________________________ --Bandwidth and Colocation provided by Easynews.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit:   http://lists.digium.com/mailman/listinfo/asterisk-dev
Comments:	By: pkempgen (pkempgen) 2007-03-19 15:29:30 Just figured that the artificially created extensions result in a somewhat "unpredictable" behavior: You never know for sure that ${EXTEN} is what you think it is because it might happen to be inside a different extension - and Dial(SIP/sw-87-user) is not what I expected :) Same thing for labels - they might end up in a different extension which breaks Goto(mylabel). By: Steve Murphy (murf) 2007-03-20 13:32:53 OK, I can tell right away, from the last comments added, that you won't particularly approve of the fix I committed! This is what I did: I modified the code generator, to start switch extensions at priority 10, instead of 1. This solves the security problem in a very minimalistic way. Without an extension "10", no-one can dial into that extension directly, as incoming caller cannot specify the priority number, and the pbx engine will look for priority 1 for starting, and it won't be there. As to the problems pointed out above, there are simple workarounds, like storing your extension name before you go into the switch. Labels, I actually spent some time on, and they should be usable in AEL, in switches. The background compiler will adjust the target extension name to make the jump successful. This bug report is for the security hole. Getting me to change the code generator to not generate extensions is a separate issue. On the one hand, if that was the only way to do it, well, you wouldn't have minded, I'm sure. Since the marshals don't like enhancement requests on mantis (it gums up the works, I guess), you can file it in the AEL2 page on the voip-info wiki, or I will, when I get a breath. By: Steve Murphy (murf) 2007-03-20 13:34:33 PS: As to starting the generated extensions at priority 10 to avoid the security hole, this was Corydon76's idea. Many thanks to Corydon76!!! He should get some karma out of this for a brilliant idea, at the least. By: pkempgen (pkempgen) 2007-03-20 13:50:18 What a billiant idea! As far as the security hole is concerned this issue seems solved to me with your "start at prio 10" fix. By: Steve Murphy (murf) 2007-03-20 14:50:26 OK, the necc. changes have been applied to 1.4 via 59069 and 59070; to trunk via 59073. I have my backport of AEL2 to 1.2 updated with this fix via 59075 (team/murf/AEL2-1.2 and team/murf/AEL2-1.2/patches) I have not touched the original AEL compiler in 1.2; it was experimental, and as per previous statements, to resolve this bug, the user is encouraged to either use the AEL2 patches for 1.2, or to upgrade to 1.4 or trunk.