Insecure argument passed to cURL

[d-z-m]

Expected Behavior

a secure download of the Miniconda install script

Current Behavior

install script is cURL'd with -k resulting in an insecure transfer and possible arbitrary code execution on my machine.

Steps to Reproduce

1. run any of these without miniconda installed:

   • wsl.sh
   • start_windows.bat
   • start_macos.sh
   • start_linux.sh

2. pwned

Possible Solution

don't curl executable things insecurely

Context

Below is a copy of the text I had put in a security advisory on both this repository and ParisNeo/lollms-webui as of early December. The response from the maintainers has been radio silence, so I am publishing the text here so that people can know/protect themselves.

Security Advisory

Summary

An unsafe command line argument being passed to cURL allows the Miniconda installer download to be MITM'd.

This downloaded script is subsequently run, potentially resulting in arbitrary code execution on user machines.

Details

Here's an example from start_linux.sh

    mkdir -p "$INSTALL_DIR"
    curl -Lk "$MINICONDA_DOWNLOAD_URL" > "$INSTALL_DIR/miniconda_installer.sh"

It passes the -k argument to cURL.

cURL man page documentation for -k:

       -k, --insecure
              (TLS SFTP SCP) By default, every secure connection curl makes is
              verified to be secure before the transfer takes place. This
              option makes curl skip the verification step and proceed without
              checking.

              When this option is not used for protocols using TLS, curl
              verifies the server's TLS certificate before it continues: that
              the certificate contains the right name which matches the host
              name used in the URL and that the certificate has been signed by
              a CA certificate present in the cert store.  See this online
              resource for further details:
               https://curl.se/docs/sslcerts.html

              For SFTP and SCP, this option makes curl skip the known_hosts
              verification.  known_hosts is a file normally stored in the
              user's home directory in the ".ssh" subdirectory, which contains
              host names and their public keys.

              WARNING: using this option makes the transfer insecure.

The operative line is at the end:

WARNING: using this option makes the transfer insecure.

Impact

All users of the following installer scripts are affected:

• wsl.sh
• start_windows.bat
• start_macos.sh
• start_linux.sh

[d-z-m]

You are worried that someone will modify the script mid download because it's not being downloaded using SSL?

It is being downloaded using SSL, but the SSL is unverified/untrusted. This means you have no reasonable expectation that the script you are downloading and executing is the one you think you are.

And you thought that this is some big thing that the maintainers needed to keep quiet vs posting a bug?

There are often different concerns at play with security issues such as this. Oftentimes, it's a balancing act between mitigation/patching and disclosure. If software is widely used enough, publishing a security issue publicly can result in bad actors moving to exploit it before a patch can be written. I opted to privately disclose it at first, but after apathy/non-respnsiveness from maintainers, I felt the best way to proceed was to publish the contents of the advisory.

BTW, pytorch binaries were compromised some time ago with a supply chain attack.. all the TLS in the world wouldn't have saved you.

Yes, that was bad. This is also bad, and needs to be fixed.

[d-z-m]

So someone is swapping SSL certs on you and THIS is gonna be the attack vector, huh?

Yes, actually. You can not appreciate that if you'd like, but it's a real attack vector.

it's a balancing act between mitigation/patching and disclosure.

lol

I don't understand your objection, can you elaborate?

[d-z-m]

I think the way you approach this is very alarmist which is why nobody replied.

Do you mean on this issue? I only created it a couple hours ago xD. I don't mean to come across as alarmist, but I do mean to convey that I think this is a serious problem.

If you're talking about the advisory: Ooba did reply in the advisory, but was dis-inclined to act/fix it, be cause they thought it enabled the installer script to work on Windows machines. I replied that at least it could be removed on the linux and macos installers, and issued a PR to that effect, but it was ignored.

It would have been much better to just make a PR to fix this simple issue and explain why it's not best practice.

I issued a PR, but it was ignored.

The likelihood of this attack is very very small and the implications of this happening are much worse beyond a project like this. How often is anyone going to be downloading miniconda at the right time?

Just because you believe it is unlikely/improbable etc. doesn't make it so. The fact remains that it is possible when it absolutely should not be.

The stars literally have to align for someone to use a script to d/l an (new) environment for freaking LLMs

I would submit that exploiting this isn't as improbable as you seem to think.

This is the ONE thing that will get them

Exactly, we're in total agreement.

[oobabooga]

As I explained in your private PR, this was added a while ago in this PR: https://github.com/oobabooga/one-click-installers/pull/9

Without this flag, the installer seems to fail to run on some Windows systems: https://github.com/oobabooga/text-generation-webui/issues/644#issuecomment-1493518391

[d-z-m]

Hi Ooba. Thanks for replying. Personally, I think that making the miniconda installer download and execution insecure for all users is a poor solution to what I think was probably a transient error(while Cloudflare rolled out some intermediate CA or something) for a very small minority of users. I'm curious to know what you think, though.

In any case, I think that the -k should be removed from the macOS and linux install scripts at a minimum. Does this seem amenable?

If you believe the -k should always be a fallback option, what do you think about the possibility of adding an interactive step in the script? Something like...if the secure download fails, the user will be re-prompted to attempt the download insecurely, and must hit enter to accept a warning message?

[oobabooga]

We can try to remove it, but I feel like that will generate many issues about how the installer is broken, everything is broken, nothing works, etc.

I need some windows tests on https://github.com/oobabooga/text-generation-webui/pull/5535 before merging.

[d-z-m]

I need some windows tests on https://github.com/oobabooga/text-generation-webui/pull/5535 before merging.

Tested start_windows.bat on Windows_Server-2022-English-Full-Base-2024.01.16 running in AWS. works 👍.

Haven't tested with WSL yet.

[d-z-m]

This had been fixed in #5535 on all platforms.

A few days later, someone surfaced with the issue on windows #5628 and the change was reverted in 7eee9e9.

Sigh. I can't stress enough how harmful it is to leave this as the default. Users are exposed to much maliciousness as a result of leaving this in.

It is, in practice, the exact same as curl | bashing a script over plain http, which everyone knows is a Bad Thing. If I opened an issue and said the installer wasn't working for me because I can't use https, would you change it to plain http?

I can't really say any more than this. Please reconsider the posture you are taking relative to this issue.

[oobabooga]

If you can think of something better send a PR. This breaks the installer on Windows for multiple users, which is not acceptable. It's not my fault if Windows can't handle a certificate verification for an HTTPS download.

[d-z-m]

If you can think of something better send a PR.

I did. I also tested it on Windows, and it worked. Perhaps these people who are having problems need to update their operating systems.

This breaks the installer on Windows for multiple users, which is not acceptable.

We have different definitions of unacceptable. In my opinion it's unacceptable to expose users to a MITM attack on an executable file.