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

CRLF/HTML entity injection with most recent version of PHPMyAdmin #16056

Closed
oldkingcone opened this issue Mar 30, 2020 · 19 comments
Closed

CRLF/HTML entity injection with most recent version of PHPMyAdmin #16056

oldkingcone opened this issue Mar 30, 2020 · 19 comments
Assignees
Labels
question Used when we need feedback from the submitter or when the issue is a question about PMA undecided
Projects

Comments

@oldkingcone
Copy link

Describe the bug

The login form does not properly escape CRLF sequences, this can lead to HTML entity injection at the very least, or reflected XSS at the very worst.

To Reproduce

Steps to reproduce the behavior:

  1. Go to 'index.php'
  2. Insert into the login fields, username, password, and the hidden field of target, and insert this value into those fields: %0D%0Astring%0D%0A and the login error will display the injected CRLF sequences, and injected url encoded entities, such as single or double quotes(double quotes are not as effective as single quotes).

Expected behavior

I expected the application to drop/filter out the CRLF sequences seeing as an issue similar was detected back in 2005.

Screenshots

proof

proof2

Server configuration

  • Operating system: Ubuntu
  • Web server: Apache
  • Database version: MySQL
  • PHP version: most recent.
  • phpMyAdmin version: most recent.

Client configuration

  • Browser: Firefox
  • Operating system: Windows 10

Additional context

None.

@carnil
Copy link

carnil commented Mar 31, 2020

CVE-2020-11441 seem to have been assigned for this issue.

@williamdes
Copy link
Member

@williamdes williamdes added the Bug A problem or regression with an existing feature label Mar 31, 2020
@williamdes williamdes added this to Needs triage in issues via automation Mar 31, 2020
@williamdes williamdes moved this from Needs triage to High priority in issues Mar 31, 2020
@ibennetch
Copy link
Member

ibennetch commented Mar 31, 2020 via email

@oldkingcone
Copy link
Author

@ibennetch I had looked for a more direct route to report this to your team. I looked on your website, I looked in the mailing lists you have listed on your website, I had even looked in this repo for a more clear way to report the issue securely. So I would appreciate the shade throwing be kept to a minimum please.

@Ekultek
Copy link

Ekultek commented Apr 1, 2020

You know what I find sad? That the developers were aware of this issue and kept it a secret instead of telling people "hey guys we kinda messed up just watch for this and this we'll get it fixed ASAP". Have we not learned from the shadow brokers that keeping issues like this a secret just causes even more problems for everyone involved?

I'm sorry but if your team was aware of the issue it should of been your number one priority to tell the general public, especially with you being an open source project. People rely on this application for security reasons and you sitting here and saying: "The security team is aware of this issue. It’s unfortunate that the reporter chose to post here instead of follow our directions for reporting a security issue privately" makes it seem like you don't care about your users safety. We all have a right to know when you guys mess up or make a mistake.

So how about you fix it, stop complaining about the way it was reported, and be thankful that someone reported it instead of taking it to a broker and selling it after going the extra two steps to make it even worse than it is. K? Thanks.

@ibennetch
Copy link
Member

@oldkingcone Sorry, I looked again through the messages to the security address and don't see yours there. The email address is the main way we look for reports, and I don't know why I haven't seen your messages.

Regardless, we're glad to get the report, of course.

@ibennetch
Copy link
Member

@Ekultek I think you misunderstood. There were two comments between the post and mine, but not any confirmation from a team member that this is being taken seriously. Nothing I said was meant to mean "We have known about this and are choosing not to fix it." but rather "We see this report and are working, but because of needing to backport fixes and determine which branches are affected you may not see a lot of discussion here."

I respectfully disagree with you, our priority when a security flaw is reported should be fixing the issue and creating a new release, not immediately announcing it publicly without any information about how bad the flaw is or how to fix it. Most projects — in fact, every open source project I know of and have worked with — handles reports by fixing sensitive security issues first rather than publicly reporting the issue immediately.

We always welcome feedback to how we can improve our processes to keep up with industry trends, if there's something more you can suggest we improve, please let me know.

@ibennetch
Copy link
Member

Also, to everyone, my tone seems to have been poorly written, I did not mean to be dismissive of @oldkingcone or unhappy about the report. As I mentioned, I'm of course glad for the report.

Sorry the tone of my message was poorly written.

I don't know why the security address didn't work but will investigate the server logs further when I have a chance.

@ibennetch
Copy link
Member

ibennetch commented Apr 1, 2020

I can't reproduce this with the steps provided. I've put in the username root%0D%0Astring%0D%0A in the username field and when I submit the form, the error seems to be properly quoted:

image

I've tried with both submitting the text directly through the form manually as reported and also by intercepting the request with a proxy. When I intercept the request and insert the query, I get the same result:

image

The returned HTML:

                <input type="text" name="pma_username" id="input_username" value="root
string
" size="24" class="textfield">

and

<div id="pma_errors"><div class="error"><img src="themes/dot.gif" title="" alt="" class="icon ic_s_error"> mysqli::real_connect(): (HY000/1045): Access denied for user 'root
string
'@'192.168.30.20' (using password: YES)</div></div>

both also seem to be properly escaped.

I've also experimented with various flavors of escaped single- and double-quotes and haven't seen a problem.

Or are you referring to the literal single quotes (') that appear in the HTML such as what you quoted from the error response:

Access denied for user '
''' echo (&amp

and so on? That isn't literally exactly what we pass to MySQL; we do some transformations on the error message before returning it to the browser.

Can you provide more information about how to reproduce this?

@oldkingcone
Copy link
Author

oldkingcone commented Apr 1, 2020

@ibennetch sure thing! I will provide more information on reproducing this. I have see similar issues like this arise in other technologies(i will not disclose here), the issue isnt SQL injection or passing it back to the database, the issue arises from the CRLF sequences being injected into the page, and the subsequent HTML entities also being injected into the page. Since I had not focused too much on getting a POC for this issue reguarding the html entity injection, i will find a payload that fits this issue, and pass it to you, sometime tomorrow, that exemplifies XSS.

As far as the literal quotes that appear in the html as highlighted by the issue, those were injected by me. The issue im more concerned about is someone injecting something like this into the page:
%22%0D%0A%0D%0A%3c%21%2d%2d%0D%0A%0D%0A

And having it rendered by the page, turning the rest of that page into a comment. leaving the attacker to rewrite the entire page to mimic the valid login for PHPMyAdmin. As this was found during a PenTest i did not have time to fully play with the payload to properly escape the html and have it display onto the page. I will get my lab setup, and get you a more fitting payload.

Refer to this screenshot:
https://user-images.githubusercontent.com/11233163/77866745-474dbf00-7202-11ea-907d-54b90ce44739.jpeg

@ibennetch
Copy link
Member

Refer to this screenshot:

I did, and I don't see anything specifically exploitable there. From what I can tell, the submitted characters are correctly encoded/escaped for submitting to MySQL, then again properly converted back to HTML for display on the error page. I look forward to seeing more when you have a chance to investigate further.

@ibennetch ibennetch added the question Used when we need feedback from the submitter or when the issue is a question about PMA label Apr 5, 2020
@carnil
Copy link

carnil commented Apr 5, 2020

@oldkingcone, @ibennetch, @williamdes From a CVE assignment point of view (note I was neither involved in requesting the CVE, just stumbled over it while looking up CVEs in the MITRE feed as part of a downstream) is this issue then valid? If not, does the CVE need to be rejected (if so this is best done by the CVE requestor or upstream at MITRE level via https://cveform.mitre.org)? (That said it was already marked as DISPUTED in the CVE entry).

@Ekultek
Copy link

Ekultek commented Apr 7, 2020

It seems that the issue is valid but they're disputing the CVE due to exploitablitiy of it @carnil

@ibennetch
Copy link
Member

@carnil thank you for that information. I actually didn't even know there was a way to reject a CVE; I've had a good relationship with the MITRE folks through requesting them for the project, but never realized that was an option.

@Ekultek I haven't disputed anything with MITRE. I'm not sure who has or why the form shows that quote.

I still don't see how this report is valid, this appears to be proper escaping all the way through. I haven't seen any part of this that looks like a bug. What we've seen are literal single-quotes ' and carriage returns in the HTML, which are valid according to the HTML specification. It's not a matter of it not being exploitable, it's a matter of feeling it's a non-issue from the beginning. The reason I've left this open so long is because we value security here and wish to properly address any reports if there's even a chance of exploit. As you can see from the PMASA history, we don't shy away from reports and have assigned CVE and PMASA ids even for minor issues that probably wouldn't have needed it.

@emanuelb
Copy link

emanuelb commented Jun 9, 2020

There is no XSS/HTML/BBCode Injection shown in this report, also in 2005 according to PMASA advisories I didn't found related issue (at least by reading the summaries)
The CRLF shown is that in HTTP response the newline chars are not encoded which in this case are not a issue because the output is in:

  • HTML error message which not showing it (it's ignored, it's not inside <pre> tag or acting like it based on CSS rules)
  • input field, as newlines are automatically stripped by design/spec.

And there is no 'HTML entity injection' either (the entities are not converted to HTML to make XSS)
The CRLF shown in the HTTP response is not a issue, how is that a problem? what sort of client will parse it in a way that will be problematic?

so basically according to everything written here the issue is invalid, the CVE for it is bogus (good that it's disputed), But the images show some other very tiny issue caused by:

  • not encoding the ' char (tiny/minor and very limited content spoofing in HTML error message as it's using ' char enclosing the output)

and the bigger issue is that (which I didn't look into how to abuse it more now, previously I found bbcode injection issue, details here: https://www.phpmyadmin.net/security/PMASA-2016-67/ )

  • missing stricter validation for user input (such as 'max_length' , 'allowed_chars')

below more explanation and reproducing steps:

There is a minor and very limited reflected content spoofing issue here in html error message:

By using value like:
root'@'192.168.10.10' (using password: YES) 'root

(it's using ' char as a way to make the output spoofed/hard-to-process, such attacks are useful against text based log systems, to make answering the question 'what user is used?' from the output to be hard (is this 2 logins? does password was used or not?))

Result is (it's trimmed at roo the t is stripped, probably char-limit?):

mysqli::real_connect(): (HY000/1045): Access denied for user 'root'@'192.168.10.10' (using password: YES) 'roo'@'192.168.30.20' (using password: NO)

which can be fixed by (all below apply):

  • escaping of ' char.
  • setting background-color attribute or strong tag for the user provided input (so it will be more clear in the UI what is the reflected user value).
  • adding more input validation for user value (by ensuring it's valid according to supported accepted values in databases MYSQL and MARIADB) such as enforcing MAX_INPUT_LENGTH for this field if possible, and allowed chars (if the databases limit them)

edit to clarify that:

  • I think that there is no need for advisory / CVE / PMASA for above issue (at least until someone show more impact from abusing the lack of validation in user input)
  • The error shown is from database, so I think it's better to investigate the possible input validation for user value instead of parsing the error message in order to correct it? (which I marked removed as solution because it's very hacky) looks like I will need to report it to MYSQL/MARIADB, not sure how they can fix it without applying Structured Logging functionality for errors, which PHP would need to add support for as well.

@williamdes williamdes added undecided and removed Bug A problem or regression with an existing feature labels Jun 9, 2020
@williamdes williamdes moved this from High priority to n/a priority in issues Jun 9, 2020
@williamdes
Copy link
Member

🏓
What should be done since this issue is not valid ?

@ibennetch
Copy link
Member

I had left this open because of the potential to improve things based on Emanuel's suggestion of performing additional input validation on the user value, however I'm hesitant to do that because we always try to be as permissive as possible with accepting values, then passing them on to MySQL/MariaDB to report details if there is a failure.

As a result, I'm closing this now.

Many thanks to @emanuelb for the diligent and in-depth research and report, and thanks also to oldkingcone for the initial report.

issues automation moved this from n/a priority to Closed Jul 15, 2021
@williamdes
Copy link
Member

Can we finish the CVE dispute, so it is closed from other trackers waiting for us to decide ?

@ibennetch
Copy link
Member

I'll reach out to MITRE, but I don't think they will close a CVE report even if it's not valid. I believe they just log that it's disputed (and since someone, not from our team, already disputed it before we were even aware it existed, I don't think there's anything more to be done).

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Used when we need feedback from the submitter or when the issue is a question about PMA undecided
Projects
issues
  
Closed
Development

No branches or pull requests

6 participants