New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple Vulnerabilities in shadowsocks #995

Open
Pixelschleuder opened this Issue Oct 13, 2017 · 11 comments

Comments

Projects
None yet
7 participants
@Pixelschleuder

Pixelschleuder commented Oct 13, 2017

X41 D-Sec GmbH Security Advisory: X41-2017-008

Multiple Vulnerabilities in shadowsocks

Overview

Confirmed Affected Versions: Latest commit 2ab8c6b on Sep 6
Confirmed Patched Versions: N/A
Vendor: Shadowsocks
Vendor URL: https://github.com/shadowsocks/shadowsocks/tree/master
Vector: Network
Credit: X41 D-Sec GmbH, Niklas Abel
Status: Public
Advisory-URL: https://www.x41-dsec.de/lab/advisories/x41-2017-008-shadowsocks/

Summary and Impact

Several issues have been identified, which allow attackers to manipulate log
files, execute commands and to brute force shadowsocks with enabled
autoban.py brute force detection. Brute force detection from autoban.py does
not work with suggested tail command. The key of captured shadowsocks
traffic can be brute forced.

Product Description

Shadowsocks is a fast tunnel proxy that helps you bypass firewalls.

Log file manipulation

Severity Rating: Medium
Confirmed Affected Versions: Latest commit 2ab8c6b on Sep 6
Confirmed Patched Versions: N/A
Vector: Network
CVE: not yet issued
CWE: 117
CVSS Score: 4.3
CVSS Vector: CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:L/A:N

Summary and Impact

Log file manipulation is possible with a manipulated hostname, sent to the
server from a client, even if shadowsocks is as quiet as possible
with "-qq".

Therefore a string like "\nI could be any log entry\n" could be sent as
hostname to shadowsocks. The server would log an additional line with
"I could be any log entry".

Workarounds

There is no workaround available, do not trust the logfiles until a patch is
released.

Command Execution

Severity Rating: High
Confirmed Affected Versions: Latest commit 2ab8c6b on Sep 6
Confirmed Patched Versions: N/A
Vector: Network
CVE: not yet issued
CWE: 78
CVSS Score: 8.5
CVSS Vector: CVSS:3.0/AV:N/AC:H/PR:L/UI:N/S:C/C:H/I:H/A:H

Summary and Impact

When the brute force detection with autoban.py is enabled, remote attackers
are able to execute arbitrary commands.

Command execution is possible because of because of line 53 "os.system(cmd)"
in autoban.py, which executes "cmd = 'iptables -A INPUT -s %s -j DROP' %
ip". The "ip" parameter gets parsed from the log file, whose contents can be
controlled by a third party sending authenticated packets.

Proof of Concept

When, a string like "can not parse header when ||ls&:\n" is sent as host
name to shadowsocks, it would end up in the logfile and lead to the
execution of "ls".
Autoban.py does not execute commands with spaces due to internal
sanitization. A requested hostname like:
" can not parse header when ||ls&:\ntouch /etc/evil.txt\nexit\ncan not parse
header when ||/bin/bash</var/log/shadowsocks.log&:\n"
could be used to work around this limitation. It writes the command
"touch /etc/evil.txt" into the logfile and executes it with
"/bin/bash</var/log/shadowsocks.log".
The exit; command is an important factor, without it an unbounded recursion
would occur leading to a DoS.

Workarounds

No workaround available, do not use autoban.py.

Lack of Bruteforce detection through autoban.py

Confirmed Affected Versions: Latest commit 2ab8c6b on Sep 6
Confirmed Patched Versions: N/A

Summary and Impact

The brute force detection autoban.py does not work at all with the suggested
tail command, suggested at
https://github.com/shadowsocks/shadowsocks/wiki/Ban-Brute-Force-Crackers.

The command "python autoban.py < /var/log/shadowsocks.log" does work, but
the suggested "nohup tail -F /var/log/shadowsocks.log | python autoban.py >
log 2>log &" does not block IP's.
The "for line in sys.stdin:" from autoban.py parses the input until there is
an end of file (EOF). As "tail -F" will never pipe an EOF into the pyhon
script, the sys.stdin will block the script forever. So the
"tail -F /var/log/shodowsocks | autoban.py" will never block anything
except itself.

Workarounds

Use python "autoban.py < /var/log/shadowsocks.log" in a cronjob. Do not use
autoban.py until the command execution issue gets fixed.

Bruteforcable shadowsocks traffic because of MD5

Confirmed Affected Versions: Latest commit 2ab8c6b on Sep 6
Confirmed Patched Versions: N/A

Summary and Impact

Shadowsocks uses no brute force prevention for it's key derivation function.

The key for shadowsocks traffic encryption is static and derived from the
password, using MD5. The password derivation is in encrypt.py in line 56 to
63: "
while len(b''.join(m)) < (key_len + iv_len):
md5 = hashlib.md5()
data = password
if i > 0:
data = m[i - 1] + password
md5.update(data)
m.append(md5.digest())
i += 1
"

MD5 should not be used to generate keys, since it is a hash function.
A proper key derivation function increases the costs for this operation,
which is a small burden for a user, but a big one for an attacker,
which performs this operation many more times. As passwords usually have
low-entropy, a good password derivation function has to be slow.

Workarounds

Use a secure password generated by a cryptographically secure random
generator. Wait for a patch that uses a password based key derivation
function like "Argon2" instead of a hash.

About X41 D-Sec GmbH

X41 D-Sec is a provider of application security services. We focus on
application code reviews, design review and security testing. X41 D-Sec GmbH
was founded in 2015 by Markus Vervier. We support customers in various
industries such as finance, software development and public institutions.

Timeline

2017-09-28 Issues found
2017-10-05 Vendor contacted
2017-10-09 Vendor contacted, replied to use GitHub for a full disclosure
2017-10-11 Vendor contacted, asked if the vendor is sure to want a full disclosure
2017-10-12 Vendor contacted, replied to create a public issue on GitHub
2017-10-13 Created public issues on GitHub
2017-10-13 Advisory release
2017-10-16 Changed command execution vulnerability

@Pixelschleuder Pixelschleuder changed the title from Multiple Vulnerabilities in ShadowSocks to Multiple Vulnerabilities in shadowsocks Oct 13, 2017

@carnil

This comment has been minimized.

Show comment
Hide comment
@Mygod

This comment has been minimized.

Show comment
Hide comment
@Mygod

Mygod Oct 14, 2017

@madeye Have a look.

Mygod commented Oct 14, 2017

@madeye Have a look.

@Yangff

This comment has been minimized.

Show comment
Hide comment
@Yangff

Yangff Oct 14, 2017

In Log file manipulation, hostname is written by dns_resolver. In order to manipulation this hostname, attacker should be able to send a legal packet contains remote domains which need the key.

If so, how is the controlled by a third party sending **unauthenticated** packets working in Command Execution? Because there is no way for them to get a unauthenticated packets process by dns_resolver (tcprelay.py#L575 ).
(it's anyway vulnerable for someone how provide public shadowsocks server though. I'm just curious how it works without knowing the key preceded)

Yangff commented Oct 14, 2017

In Log file manipulation, hostname is written by dns_resolver. In order to manipulation this hostname, attacker should be able to send a legal packet contains remote domains which need the key.

If so, how is the controlled by a third party sending **unauthenticated** packets working in Command Execution? Because there is no way for them to get a unauthenticated packets process by dns_resolver (tcprelay.py#L575 ).
(it's anyway vulnerable for someone how provide public shadowsocks server though. I'm just curious how it works without knowing the key preceded)

@Akkariiin

This comment has been minimized.

Show comment
Hide comment
@Akkariiin

Akkariiin Oct 14, 2017

About the Bruteforcable shadowsocks traffic because of MD5

SS family use password to encrypt data flow and init random generator. But never save password.
And SS family depends the same pre-shared password to make server and client have same state machine .
So, it needs same password bytes that are hashed from user password. Don't need any salt and don't care whatever can revert user password from password bytes or not. But it need that every time hash the user password can get same password bytes output.

Specially, in some places, user password may direct use to do something not be hashed.

So, I don't think that is a security issue.


And in the database use case, (we call it as airport), user password generate by random, and save in database as plain text for next time to read into system. Maybe it become a issue in this case that when save in database.

But in this case still cannot use argon2 , because it cannot recovery password from bytes.

Akkariiin commented Oct 14, 2017

About the Bruteforcable shadowsocks traffic because of MD5

SS family use password to encrypt data flow and init random generator. But never save password.
And SS family depends the same pre-shared password to make server and client have same state machine .
So, it needs same password bytes that are hashed from user password. Don't need any salt and don't care whatever can revert user password from password bytes or not. But it need that every time hash the user password can get same password bytes output.

Specially, in some places, user password may direct use to do something not be hashed.

So, I don't think that is a security issue.


And in the database use case, (we call it as airport), user password generate by random, and save in database as plain text for next time to read into system. Maybe it become a issue in this case that when save in database.

But in this case still cannot use argon2 , because it cannot recovery password from bytes.

@Yangff

This comment has been minimized.

Show comment
Hide comment
@Yangff

Yangff Oct 14, 2017

@Akkariiin I don't think this attack would be practical for anyone. However it can be used for attacking some specific people if they are known to use shadowsocks and have a weak password. The point here is shadowsocks is not designed for security but for obscuring.

@Pixelschleuder 's attack is brute force guessing the weak password (or use a dictionary). In this attack, the packets (packets with IV) could be collected and decrypted offline.

For the MD5 case, there are two weaknesses.

  1. the attacker can calculate a password-key table (the same password get same key for everyone in EVP_BytesToKey) to accelerate attack.
  2. MD5 runs fast and cost little.

And Key Derivation algorithm, like argon2, runs slowly and costly and introduce salt.

Yangff commented Oct 14, 2017

@Akkariiin I don't think this attack would be practical for anyone. However it can be used for attacking some specific people if they are known to use shadowsocks and have a weak password. The point here is shadowsocks is not designed for security but for obscuring.

@Pixelschleuder 's attack is brute force guessing the weak password (or use a dictionary). In this attack, the packets (packets with IV) could be collected and decrypted offline.

For the MD5 case, there are two weaknesses.

  1. the attacker can calculate a password-key table (the same password get same key for everyone in EVP_BytesToKey) to accelerate attack.
  2. MD5 runs fast and cost little.

And Key Derivation algorithm, like argon2, runs slowly and costly and introduce salt.

@PeterCxy

This comment has been minimized.

Show comment
Hide comment
@PeterCxy

PeterCxy Oct 14, 2017

@Yangff What if a malicious program / web page on the user's computer detects the Shadowsocks client and then tries to send malicious packet through the Shadowsocks client?

PeterCxy commented Oct 14, 2017

@Yangff What if a malicious program / web page on the user's computer detects the Shadowsocks client and then tries to send malicious packet through the Shadowsocks client?

@Akkariiin

This comment has been minimized.

Show comment
Hide comment
@Akkariiin

Akkariiin Oct 15, 2017

@Yangff
like follow line that you are say :
The point here is shadowsocks is not designed for security but for obscuring.

I dont think that make the passwd hash stronger are necessary in SS family.

And if attacker can direct calc passwd binary from traffic flow (that passwd binary hashed from user passwd) , The attacher dont need to revert hash the passwd binary to user passwd, it can direct decrypt all the data from next traffic use the passwd binary . (BTW: in this case, this means attacker can break AES or ChaCha20 encrypt method, and the encrypt method that user used is not have Forward Secrecy. )

And nobody can use any technical method to save a weak password user if attacker known the ss server's port and the config only not include passwd. ( In this case, attacker can direct block the IP or port to let SS system cannot work. )

And if one real need Cryptography security, she can use another Perfect Cryptography Security tunnel over the SS. the SS family primary focus on Feature Hidden not Cryptography security. And in my view, Personal Safety/Security need many work, not only depends on a SS tunnel.

Akkariiin commented Oct 15, 2017

@Yangff
like follow line that you are say :
The point here is shadowsocks is not designed for security but for obscuring.

I dont think that make the passwd hash stronger are necessary in SS family.

And if attacker can direct calc passwd binary from traffic flow (that passwd binary hashed from user passwd) , The attacher dont need to revert hash the passwd binary to user passwd, it can direct decrypt all the data from next traffic use the passwd binary . (BTW: in this case, this means attacker can break AES or ChaCha20 encrypt method, and the encrypt method that user used is not have Forward Secrecy. )

And nobody can use any technical method to save a weak password user if attacker known the ss server's port and the config only not include passwd. ( In this case, attacker can direct block the IP or port to let SS system cannot work. )

And if one real need Cryptography security, she can use another Perfect Cryptography Security tunnel over the SS. the SS family primary focus on Feature Hidden not Cryptography security. And in my view, Personal Safety/Security need many work, not only depends on a SS tunnel.

@Yangff

This comment has been minimized.

Show comment
Hide comment
@Yangff

Yangff Oct 15, 2017

@Akkariiin
The attack is based on the complexity of key space is not keysize but size of password space which is much more smaller. And would be practical if we can get key from password easily.

So, it's not necessary to break AES or revert a hash function. A possible attack may work like this:

  1. Assumption: We can identify few shadowsocks packets and we have one packet contains IV. And it's using a weak password.
  2. enumerate possible original password P
  3. calc traffic key K by using EVP_BytesToKey(P), get IV from packet
  4. decrypt packets and check theirs validity

MD5 is fast (also encryption algorithms are fast), so the step 2 and 3 can be done pretty fast. And attackers can use precomputed table, GPU and ASIC to speed it up.

But I think for someone who use 8 digits password, their is no way to protect them...

Yangff commented Oct 15, 2017

@Akkariiin
The attack is based on the complexity of key space is not keysize but size of password space which is much more smaller. And would be practical if we can get key from password easily.

So, it's not necessary to break AES or revert a hash function. A possible attack may work like this:

  1. Assumption: We can identify few shadowsocks packets and we have one packet contains IV. And it's using a weak password.
  2. enumerate possible original password P
  3. calc traffic key K by using EVP_BytesToKey(P), get IV from packet
  4. decrypt packets and check theirs validity

MD5 is fast (also encryption algorithms are fast), so the step 2 and 3 can be done pretty fast. And attackers can use precomputed table, GPU and ASIC to speed it up.

But I think for someone who use 8 digits password, their is no way to protect them...

@Akkariiin

This comment has been minimized.

Show comment
Hide comment
@Akkariiin

Akkariiin Oct 15, 2017

@Yangff Good !!!
This method I did not think before .

If attacker knows that this is a SS flow and knowledge the right package structure (this means know the right method and protocol config) , the attacker can make a offline attack environment to try all the possible key with any number of computer time. And use argon2 only let attacker wast more time to find that. I think that this like an another form of the Choose Plaintext Attack.

Akkariiin commented Oct 15, 2017

@Yangff Good !!!
This method I did not think before .

If attacker knows that this is a SS flow and knowledge the right package structure (this means know the right method and protocol config) , the attacker can make a offline attack environment to try all the possible key with any number of computer time. And use argon2 only let attacker wast more time to find that. I think that this like an another form of the Choose Plaintext Attack.

shell909090 added a commit to shell909090/shadowsocks that referenced this issue Oct 19, 2017

fix one of issues:
shadowsocks#995
Command Execution

using list instead of string will make program safty.

shell909090 added a commit to shell909090/shadowsocks that referenced this issue Oct 20, 2017

fix issue:
shadowsocks#995
Command Execution

use list instead of string, prevent injection attack.

shell909090 added a commit to shell909090/shadowsocks that referenced this issue Oct 20, 2017

@baimafeima

This comment has been minimized.

Show comment
Hide comment
@baimafeima

baimafeima Jan 7, 2018

Is this completely patched?

baimafeima commented Jan 7, 2018

Is this completely patched?

@Pixelschleuder

This comment has been minimized.

Show comment
Hide comment
@Pixelschleuder

Pixelschleuder Jan 11, 2018

No, there are still open issues.

Pixelschleuder commented Jan 11, 2018

No, there are still open issues.

mengskysama added a commit that referenced this issue Feb 19, 2018

use list instead of string, prevent injection attack. (#1009)
* fix issue:
#995
Command Execution

use list instead of string, prevent injection attack.

heroism1993 added a commit to heroism1993/shadowsocks that referenced this issue Sep 12, 2018

use list instead of string, prevent injection attack. (shadowsocks#1009)
* fix issue:
shadowsocks#995
Command Execution

use list instead of string, prevent injection attack.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment