Data Source Name Injection in pingcap/tidb

Valid

Reported on

Dec 26th 2021


Description

TiDB Importer uses Go MySQL Driver for connecting to MySQL servers. This driver utilizes Data Source Name (DSN) strings for describing database connections with the following format:

username:password@protocol(address)/dbname?param=value

The driver has a built-in protection against LOCAL INFILE requests. To access the requested file, set the allowAllFiles parameter to true in the DSN string when connecting to the MySQL server.

Steps to Reproduce

To test the vulnerability, I need to (go) build TiDB importer command first in it.

$ git clone https://github.com/pingcap/tidb
$ cd tidb/cmd/importer
$ go build .

Next, let's set up the software which we will use as a rogue MySQL server: the Rogue-MySql-Server script by allyshka. It's a fork of the original rogue server by Gifts with additions to support modern MySQL servers.

$ wget -q https://github.com/allyshka/Rogue-MySql-Server/raw/master/roguemysql.php

Launch the roguemysql.php script and specify the path to the file you are interested in.

Screenshot_2021-12-26_19-54-35

The last step is to specify our server address in the config.toml file. Fill the required fields and don't forget to put the ?allowAllFiles=true& string after the database name.

Screenshot_2021-12-26_19-53-05

And finally, execute the importer command with the previous config file, you will get your file's content

$ ./importer -config config.toml

Screenshot_2021-12-26_20-03-56

The TiDB importer has connected to the rogue MySQL server, which has requested the /etc/passwd file to be read, and the TiDB importer has transferred this file's contents to us!

Impact

This issue may lead to arbitrary file read.

Occurrences

We are processing your report and will contact the pingcap/tidb team within 24 hours. 2 years ago
We have contacted a member of the pingcap/tidb team and are waiting to hear back 2 years ago
We have sent a follow up to the pingcap/tidb team. We will try again in 7 days. 2 years ago
pingcap/tidb maintainer
2 years ago

Maintainer


The vulnerability you submitted has been successfully reproduced. Based on our assessment, we believe that this issue may lead to potential security risks and should be treated as a vulnerability. Thanks for your research work.

And we still have a few questions about this vulnerability. The first is about the scoring of the vulnerability. We use the CVSS 3.1 standard to score vulnerabilities. But the result is different from the score in the report. The scoring vector we use is as follows: CVSS:3.1/AV:L/AC:L/PR:H/UI:R/S:U/C:H/I:N/A:N (see this link for details), and the final score we get is 4.2 (6.8 in the report). If we have any misunderstanding about the impact of the vulnerability, please let us know. Another question is how to fix this vulnerability? Do you have any suggestions?

We have sent a second follow up to the pingcap/tidb team. We will try again in 10 days. 2 years ago
We have sent a third and final follow up to the pingcap/tidb team. This report is now considered stale. 2 years ago
Dwi Siswanto
a year ago

Researcher


Hi,

Sorry for the delay. I don't understand why this doesn't ping me the notification.

Regarding how to mitigate this issue, it's actually quite simple. Because this vulnerability inherits from improper input validation, so any untrusted input must be validated before being concatenated into a DSN string.

In this case, validating the database name with the pattern as ^[0-9a-zA-Z$_]+$ should suffice, or practically use url.QueryEscape.

Dwi Siswanto
a year ago

Researcher


@maintainter - Pretty sure that you should be able to confirm this vulnerability by acknowledging it.

@admin - I don't understand that the maintainer has replied to this report, but there is still a "follow-up" status. Can you please inform the @maintainer(s)?

Jamie Slome
a year ago

Admin


The maintainer will get an e-mail ping because of the messages you and I am creating here 👍

pingcap/tidb maintainer
a year ago

Maintainer


Hi @Dwi, thanks for your reply. Can you update the CVSS score and select a more accurate vulnerability type? This issue doesn't seem to be a SQL injection.

Dwi Siswanto modified the report
a year ago
Dwi Siswanto modified the report
a year ago
Dwi Siswanto
a year ago

Researcher


Hi @maintainer, sorted as CWE-134.

I list occurrences based on cases that have been tested and I'm afraid it will be a relic of future discoveries, so I believe that vulnerable codes are not the only ones I listed - see https://cs.github.com/?scopeName=All+repos&scope=&q=repo%3Apingcap%2Ftidb+%22%40tcp%22. Thus, I changed the availability score to low.

Let me know what you think.

pingcap/tidb maintainer
a year ago

Maintainer


Hi @Dwi, thanks for your update. Since a successful exploit can only result in arbitrary file read, in my opinion, this issue does not affect the availability of the server. (I find the definition of availability in section 2.3.3 in this document: https://www.first.org/cvss/v3.1/specification-document)

Dwi Siswanto modified the report
a year ago
Dwi Siswanto
a year ago

Researcher


Sorted, if you think you've reviewed the presence of similar occurrences.

pingcap/tidb maintainer validated this vulnerability a year ago

@Dwi, thanks for your quick reply, I've confirmed this issue.

Dwi Siswanto has been awarded the disclosure bounty
The fix bounty is now up for grabs
The researcher's credibility has increased: +7
Dwi Siswanto submitted a
a year ago
Dwi Siswanto
a year ago

Researcher


Hi there, I just submitted a patch for this issue.

Before

[mysql] 2022/08/30 14:09:15 packets.go:37: unexpected EOF
[2022/08/30 14:09:15.770 +07:00] [FATAL] [main.go:80] ["invalid connection"] [stack="main.main\n\t/tmp/tidb/cmd/importer/main.go:80\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:250"]

After (03092fa481e94d3c0f626f5953c92f344dfbd553)

[mysql] 2022/08/30 14:09:38 packets.go:37: unexpected EOF
[2022/08/30 14:09:38.530 +07:00] [FATAL] [main.go:80] ["local file '/etc/passwd' is not registered"] [stack="main.main\n\t/tmp/tidb/cmd/importer/main.go:80\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:250"]
pingcap/tidb maintainer
a year ago

Maintainer


That's great. Can you send a Pull Request? So that we can review and merge this fix into master.

Dwi Siswanto
a year ago

Researcher


Opened at #37489. Also, can you please test them by any chance? :)

pingcap/tidb maintainer
a year ago

Maintainer


Good job, I will contact the feature owner to review these fixes.

We have sent a fix follow up to the pingcap/tidb team. We will try again in 7 days. a year ago
We have sent a second fix follow up to the pingcap/tidb team. We will try again in 10 days. a year ago
We have sent a third and final fix follow up to the pingcap/tidb team. This report is now considered stale. a year ago
Dwi Siswanto
a year ago

Researcher


Hi there, seems like the patch d0376379d615cc8f263a0b17c031ce403c8dcbfb already rolled out. Can you please give an update on this? Thanks.

lance6716 marked this as fixed in 6.4.0, 6.1.3 with commit d03763 a year ago
lance6716 has been awarded the fix bounty
This vulnerability has been assigned a CVE
db.go#L321 has been validated
lance6716
a year ago

Maintainer


Hi @dwisiswant0, I'm just new to here, not sure how to choose the right action. Currently the fixing patch is included in the version v6.4.0, but v6.4.0 is planned to be release some weeks later. Maybe we should click "Publish" when v6.4.0 is released?

Pavlos
a year ago

Admin


Yeah exactly :)

Dwi Siswanto
a year ago

Researcher


Oh, I thought it's been released. If so, let's schedule the advisory to be released 24 hours after the fix is released (for people to start to upgrade, mainly to verify the new release hasn't caused any other problems). Thanks, @lance6716.

Dwi Siswanto
10 months ago

Researcher


Hi there, since this was rolled out in version 6.4.0, I believe that there are release notes left behind for it. It's no less important to notify tidb-server users to be aware of that, can you please update the release changelog for that version? I initiated it in this #37489 PR (check the release note section) and feel free to adapt.

Changelog aside, can we agree to disclose this now? Since it's been released 4 days ago, which might help boost the awareness I said.

Thanks. (:

lance6716 published this vulnerability 10 months ago
lance6716 gave praise 10 months ago
Thanks for reminding me. I'll talk to my security and documentation colleague about the security note. I remember we have a separate web page that listing releated CVEs, not in release note page, but currently I can not find it :P
The researcher's credibility has slightly increased as a result of the maintainer's thanks: +1
Dwi Siswanto
10 months ago

Researcher


Thank you @lance6716 & the TiDB team. ♡

to join this conversation