schollz / croc

Easily and securely send things from one computer to another :crocodile: :package:
https://schollz.com/software/croc6
MIT License
26.78k stars 1.07k forks source link

fix: shared secret should be read from environmental variable #701

Closed schollz closed 1 month ago

schollz commented 1 month ago

To fix this, the shared secret should be read from an environment variable

Fixes #598

schollz commented 1 month ago

Fixes https://nvd.nist.gov/vuln/detail/CVE-2023-43621

nateify commented 1 month ago

Could we possibly get a flag introduced to revert to pre-9.1.16 behavior? This isn't a concern for me on single-user systems.

schollz commented 1 month ago

@nateify that's fine by me. what should the flag be?

nateify commented 1 month ago

@nateify that's fine by me. what should the flag be?

I think something like --generate-code or --generate-secret

adryd325 commented 1 month ago

an environment variable to revert behavior would be nice too

lazylua commented 1 month ago

TBH I don't know how to use croc anymore :'(

I tried

all the time , getting creating securing channel...2024/05/23 13:08:42 could not secure channel

$ CROC_SECRET=23423423 croc recieve
securing channel...2024/05/23 13:17:27 room (secure channel) not ready, maybe peer disconnected
fanxianluotuo commented 1 month ago

I found that croc became completely unusable after the upgrade.

Both the sender and the receiver are using version v9.6.17. On the receiver side, I have tried

export CROC_SECRET="my-code-phrase"
croc --relay myhost:9009 --pass mypass my-code-phrase

or

croc --relay myhost:9009 --pass mypass
Enter receive code: my-code-phrase

The result is always: securing channel...2024/05/23 17:07:04 room (secure channel) not ready, maybe peer disconnected

Delete my self-host relay config didn't help.

schollz commented 1 month ago

@lazylua @fanxianluotuo make sure both clients are v9.6.17

I can replicate your errors, but only if one client is <=v9.6.15.

also make an issue for your issue please to compartmentalize discussions

schollz commented 1 month ago

I updated to v10 to signal a breaking change: https://github.com/schollz/croc/releases

jonasjelonek commented 1 month ago

@schollz I'd also like to have the old behaviour somewhat back.
What about moving out the code argument out of the send command into the global options, keep its usage if the send command is used and if we want to receive, we can reuse this option for that purpose? The relay command would probably just ignore this option

schollz commented 1 month ago

those arguments are still on the command line and show up on ps though?

jonasjelonek commented 1 month ago

sure, that would be the issue that we reintroduce the vulnerability in case you use this option. As @nateify mentioned this may not be an issue if you're on a single-user system. But I would follow your suggestion, since I also understand the issue with this vulnerability and, though it may take some time to get used to it, would be fine with giving the code via environment variable or in the croc prompt.

but what I don't really understand currently, is macOS not affected by this vulnerability?

schollz commented 1 month ago

yes good point, macOS is, fixed in latest v10.0.5

also added --classic with disclaimer and opt-in only to get the classic mode if you accept the risk

DerDennisOP commented 1 month ago

When just providing the classic flag, the given error message is confusing:

croc --classic hydra.zip
Classic mode is currently DISABLED.

Please note that enabling this mode will make the shared secret visible
on the host's process list when passed via the command line. On a
multi-user system, this could allow other local users to access the
shared secret and receive the files instead of the intended recipient.

Do you wish to continue to enable the classic mode? (Y/n) y

Classic mode ENABLED.

To send and receive, use the code phrase:

  Send: croc send --code *** file.txt

  Receive: croc ***
schollz commented 1 month ago

@DerDennisOP why

jonasjelonek commented 1 month ago

@DerDennisOP I guess you're using croc the wrong way. The --classic flag should be used on the receive side, the sender side is just used as before.

DerDennisOP commented 1 month ago

I thought that the classic flags needs to be set on both sides (sending and receiving) and the error message when trying to use --classic on the sending sides suggests to use the none classic sending command.

bb010g commented 3 weeks ago

Using an environment variable doesn't help much. A process can parse the plaintext out of /proc/$CROC_PID/environ instead of /proc/$CROC_PID/cmdline now.

schollz commented 2 weeks ago

@bb010g using an environmental variable protects against other users on a multiuser system since /proc/$CROC_PID/environ is not available to other users.