wdoekes / asterisk-chan-dongle

chan_dongle channel driver for Huawei UMTS cards, works with Asterisk 14+
Other
296 stars 104 forks source link

Currently published dialplan sample allows SMS sender to execute arbitrary commands on asterisk host. #156

Closed qtlin closed 2 years ago

qtlin commented 2 years ago

To reproduce: send yourself a test SMS with a single quote and its contents never mail out. Which code needs to be modified to escape single quotes if found in message text?

Executing [sms@from-trunk-to-sv:2] System("Local/sms@from-trunk-to-sv-00000006;1", "echo '2022-01-10 22:39:15 - dongle1 - InfoSMS Test single quote: don't do it!' | mail -r from@example.org -s "you've got new sms" to@example.org") in new stack

wdoekes commented 2 years ago

This is not a chan_dongle issue. This is a shell escaping question:

$ echo 'abc'
abc

$ echo "abc's"
abc's

$ echo 'abc'\''s'
abc's
qtlin commented 2 years ago

Please keep open until a resolution is reached. My question is that default configuration does not do any shell escapes and allows all sorts of attacks like executing arbitrary shell commands on asterisk host.

miopa commented 2 years ago

There is no default configuration. Are you referring to this example? https://github.com/wdoekes/asterisk-chan-dongle/blob/master/README.md#here-is-an-example-for-the-dialplan

qtlin commented 2 years ago

Yes, I am. The example dialplan allows SMS sender to execute arbitrary commands on asterisk host.

qtlin commented 2 years ago

A polite reminder @wdoekes to reopen and remove "invalid" flag. Currently published dialplan sample allows SMS sender to execute arbitrary commands on asterisk host.

wdoekes commented 2 years ago

So, you're referring to the following dialplan? image

qtlin commented 2 years ago

Publishing a WARNING is not an excuse for a horrible code sample. Why don't you replace that System() call with something very sane and remove a WARNING?

exten => sms,n,System(echo '${BASE64_ENCODE(${STRFTIME(${EPOCH},,%Y-%m-%d %H:%M:%S)} - ${DONGLENAME} - ${CALLERID(num)}: )}${SMS_BASE64}'  | base64 -d >> /var/log/asterisk/sms.txt)
wdoekes commented 2 years ago

Yuck. You're making an already horrible code sample even worse. I did not create that monstrosity, and I will not take any blame for it.

Try the Set(FILE(,,,a)=...) function to append lines instead..

If you test your updated dialplan before filing the PR, I can merge your changes more quickly.


P.S. Concatenating base64 like you do in your example will not be accepted by every decoder. Your base64(1) binary may decode it, or may decide to stop after the first blob, or throw an error. See for instance how python handles it:

>>> b64decode(b64encode(b'abc\n') + b64encode(b'def\n'))
b'abc\n'
qtlin commented 2 years ago

I am not using file, I am piping directly to mail. Save to file is from your example. I just updated a part on the left of >> to match existing. This

exten => sms,n,System(echo '${BASE64_ENCODE(${DONGLENAME} - ${CALLERID(num)}: )}${SMS_BASE64}'  | base64 -d | mail -r asterisk@sender.example.org -s "you've got new sms" sms@recipient.example.com )

has been copied from a working system, all special characters are delivered to a recipient. I don't like "monstrosities" either, if you know of something shorter and equally reliable I am all ears.

qtlin commented 2 years ago
>>> b64decode(b64encode(b'abc\n') + b64encode(b'def\n'))
b'abc\n'

apparently python decoder is broken, GNU base64 works just fine, I did an extensive testing before implementing my change.

$ /usr/bin/echo -n `/usr/bin/echo -e 'abc\n' | base64``/usr/bin/echo -e 'def\n' | base64` | base64 -d
abc

def

$