[Home]

Summary:ASTERISK-09846: Possible injection attack in dialplan - time consuming to program around.
Reporter:Nick Barnes (bcnit)Labels:
Date Opened:2007-07-10 03:14:20Date Closed:2011-06-07 14:02:38
Priority:MajorRegression?No
Status:Closed/CompleteComponents:PBX/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:
Because wildcard pattern matching in the dial plan (with the '.' character) matches against any character, it is not easy to ensure that an extension (or caller ID) is entirely numeric. If it is not entirely numeric, then it is not easy to ensure that it doesn't contain any dodgy characters which may then be used to injection attack the shell (System()) or database (MySQL()).

For example, assume the following (grossly oversimplified) dialplan:

[process-number]
; A simple subroutine which lets us log whatever is passed to us
exten => _X.,1,NoOp(Logging ${EXTEN})
exten => _X.,n,System(/etc/asterisk/bin/simplelogger ${EXTEN})
exten => _X.,n,NoOp(done logging)
exten => _X.,n,NoOp(do whatever else needs to be done.........)


Now, if we issue a:
 Goto(process-number,${CALLERID(number)},1)
We may expect it to work OK.
However, if ${CALLERID(number)} contains "0\;rm -rf /\;#", we're in big trouble.

OK, so that's the idiot's way of doing things and all the variables should be checked before we do anything with them.

Unfortunately, there's no easy way to perform these checks.

A large portion of these potential problems could be solved if there was a wildcard pattern matching character which matched only numbers.

So, if we assume the '%' character is used as a numeric only wildcard, we could change the code above to:

[process-number]
; A simple subroutine which lets us log whatever is passed to us
exten => _X%,1,NoOp(Logging ${EXTEN})
exten => _X%,n,System(/etc/asterisk/bin/simplelogger ${EXTEN})
exten => _X%,n,NoOp(done logging)
exten => _X%,n(donotlog),NoOp(do whatever else needs to be done.........)
exten => i,1,Goto(donotlog)

Which is nice and simple and also safe.

I appreciate that the example above is very simple, but it does illustrate the point. Unfortunately, all the checking MUST take place in Asterisk (we can't trust the call to the external application if it isn't) and Asterisk doesn't provide the means to do this (easily).


****** ADDITIONAL INFORMATION ******


At the moment, I'm checking dialled number and caller ID number as follows:

; Dialled number
exten => _0ZX,1,NoOP()
exten => _0ZXX,1,NoOP()
exten => _0ZXXX,1,NoOP()
exten => _0ZXXXX,1,NoOP()
exten => _0ZXXXXX,1,NoOP()
exten => _0ZXXXXXX,1,NoOP()
exten => _0ZXXXXXXX,1,NoOP()
exten => _0ZXXXXXXXX,1,NoOP()
exten => _0ZXXXXXXXXX,1,NoOP()
exten => _0ZXXXXXXXXXX,1,NoOP()
exten => _0ZXXXXXXXXXXX,1,NoOP()
;
; Now check caller ID in the same way.
exten => _X./,2,NoOP()
exten => _X./_X,2,NoOP()
exten => _X./_XX,2,NoOP()
exten => _X./_XXX,2,NoOP()
exten => _X./_XXXX,2,NoOP()
exten => _X./_XXXXX,2,NoOP()
exten => _X./_XXXXXX,2,NoOP()
exten => _X./_XXXXXXX,2,NoOP()
exten => _X./_XXXXXXXX,2,NoOP()
exten => _X./_XXXXXXXXX,2,NoOP()
exten => _X./_XXXXXXXXXX,2,NoOP()
exten => _X./_XXXXXXXXXXX,2,NoOP()
exten => _X./_XXXXXXXXXXXX,2,NoOP()
exten => _X./_XXXXXXXXXXXXX,2,NoOP()
exten => _X./_XXXXXXXXXXXXXX,2,NoOP()
exten => _X./_XXXXXXXXXXXXXXX,2,NoOP()


This works very well, but is obviously impractical for checking other variables.

The other option would be to allow regular expressions!
Comments:By: Harry Roberts (harryr) 2007-07-10 05:33:03

Can you not use something like?

exten => _X.,n,GotoIf(${REGEX("^[0-9]+$" ${EXTEN})} != 1 ? 50)
; Application logic
; Hangup
exten => _X.,50,Cepstral(Eat my shorts)
By: Nick Barnes (bcnit) 2007-07-10 06:05:34

No!

where ${EXTEN} is entirely numeric,

${REGEX("^[0-9]+$" ${EXTEN})}

returns 0.

${REGEX("^[0-9]+" ${EXTEN})}

does return 1, but that will also return 1 if ${EXTEN} == "123\;rm -rf /\;#"

This would seem to imply that there are extra characters on the end, however...

${REGEX("^[0-9]+" ${EXTEN}.*)}

returns 0.

That's about the extent of my understanding of regular expressions!
By: Nick Barnes (bcnit) 2007-07-10 06:22:08

On the subject of REGEX()....

It seems that you can't use a '$' within the regular expression!

So,
 ${REGEX("^[0-9]+$" ${EXTEN})}
doesn't work. However,
 Set(dollar=$)
 ${REGEX("^[0-9]+${dollar}" ${EXTEN})}
does work.
By: Nick Barnes (bcnit) 2007-07-10 06:44:41

I take your point about using REGEX(), however, I am wary of using anything to check a value which requires passing that value as an argument.

Where the call to REGEX() is defined as 'REGEX("<regular expression>" <data>)', is there any possible value of <data> which could break this and/or allow an injection attack?
By: Harry Roberts (harryr) 2007-07-10 07:11:27

Iirc, the ${EXTEN} part is passed raw to the regex function, and the function then expands that when calling one of the parsing argument macros.

So if you had:

${EXTEN} = '  ) die asterisk die'

The REGEX command won't be called as 'REGEX("regex" "  ) die asterisk die', it will be passed correctly to the function which will then have '  ) die asterisk die' as the value to match against.
By: Nick Barnes (bcnit) 2007-07-10 07:17:03

OK. Thank you.

If it is the case that the REGEX() function is bulletproof, then can I request that this bug is downgraded to a feature request?

I still think it would be useful to be able to wildcard-match numbers only.
By: Russell Bryant (russell) 2007-07-10 11:25:39

For the case of MySQL, there is a SQL_ESC() dialplan function, if you need to sanitize input from SQL injection.  Anyway, this is a configuration issue ...

Also, we do not accept feature requests through the bug tracker, so I'm going to go ahead and close this out.