windows: clear sensitive command-line options from process list

[vszakats]

Writing to argv is not effective on Windows, but there is an alternate
method to modify the command-line as seen here:
https://github.com/lordmulder/cURL-build-win32/blob/2e9d3818baf8f7b1183331750b7b33152dd6ee82/patch/curl_tool_doswin.diff

Try a patch that's re-creating the full command-line string from each
argument except sensitive ones and writing it back to the command-line
buffer.

The known issue is the use of banned func _tcscat().

---

• replace cats.
• look into suspect GHA/windows Unicode job issues. Timing out with "too large log" or similar.
  https://github.com/curl/curl/actions/runs/11953800552/job/33325198409?pr=15620
  https://github.com/curl/curl/actions/runs/11953800552/job/33325199361?pr=15620
  Seems to be mingw-w64 + c-ares builds (Unicode and non-Unicode):
  https://github.com/curl/curl/actions/runs/11964350233/job/33356658887
  Doh, I missed free().
• figure out which Windows targets support it. (MSVS 2008, 2010, 2013, mingw-w64 6.4.0, do not)
• mingw-64 32-bit: 1264 Segmentation fault bld/src/curl.exe --disable --version: https://github.com/curl/curl/actions/runs/11945298830/job/33297864984?pr=15620#step:20:14

[jay]

I don't like this, I think it's unnecessary and adds complexity. Windows users can't see other users' command lines without privilege, unlike other operating systems. I use Process Explorer a lot and find the command lines useful to see what's running. I can't think of another application on my system which modifies command lines like this.

Ref: #10888 (comment)

[vszakats]

Calling this malware doesn't help. There is nothing malicious in hiding secrets from public buffers.

Having secrets visible for other processes running under the same or any higher privileged accounts is a legit concern I think. Is it common that someone wants to recover (or see) their own credentials via Process Explorer? All the rest of the command-line is still visible after this patch. (Contrary to the linked patch which hides it entirely.)

I agree it's not a beauty but that's not unlike other Windows-specific quirks already rolled within the source. It probably can be tidied-up by moving the complexity inside tool_doswin.

[vszakats]

The HTTP/3 wolfSSL CMake failure is unrelated and possibly caused by a recent upstream commit (between be70bea687526a51e3d751d425bbaaa412b451ee and c06f65a8ace311667d9b9d7fd320b6b25f8b1bf8: https://github.com/wolfSSL/wolfSSL/compare/be70bea687526a51e3d751d425bbaaa412b451ee..c06f65a8ace311667d9b9d7fd320b6b25f8b1bf8) or the image update (from 20241112.1.0 to 20241117.1.0), after which these two features are no longer detected in curl CMake builds:
HAVE_WOLFSSL_BIO, HAVE_WOLFSSL_FULL_BIO. Then building without HTTPS-proxy and failing the corresponding pytests.

/home/runner/work/curl/curl/CMakeFiles/CMakeScratch//CheckSymbolExists.c:8:19: error: ‘wolfSSL_BIO_new’ undeclared (first use in this function); did you mean ‘wolfSSL_CTX_new’?
/home/runner/work/curl/curl/CMakeFiles/CMakeScratch//CheckSymbolExists.c:8:19: error: ‘wolfSSL_BIO_set_shutdown’ undeclared (first use in this function); did you mean ‘wolfSSL_set_shutdown’?

[jay]

To be fair, in the other thread I said modifying the command line is something malware does, and that isn't saying your code is malware.

If a user puts secrets in command lines then I don't think it's the responsibility of any program to have to remedy that. Also, I was opposed to this behavior in general for all operating systems in part because it's not completely effective. As long as operating systems allow it adversaries can poll command lines for secrets.

[vszakats]

To be fair, in the other thread I said modifying the command line is something malware does, and that isn't saying your code is malware.

Thanks, no hard feelings!

If a user puts secrets in command lines then I don't think it's the responsibility of any program to have to remedy that. Also, I was opposed to this behavior in general for all operating systems in part because it's not completely effective. As long as operating systems allow it adversaries can poll command lines for secrets.

Fair point. This is indeed not a 100% solution, but rather something that helps
in likely many practical cases.

A while ago spent an unjustifiable amount of time converting all scripts to
not pass secrets via the command-line. It was complicated, non-obvious,
full of gotchas, even tool bugs, and after several iterations, still not a 100%
solution. Later got a message that my method is in fact broken, or may be
broken, depending. This was in POSIX/bash shells, and I imagine doing the
same with batches is possibly worse. I don't know about PowerShell.

I think making the common use-case safer, may help this. While also making
it clear that neither this or the write-arg solution replaces the superior methods.

All that said, I'm curious what is the safe way to pass secrets to a command-line
tool without exposing it to the command-line, envs, saving it to disk, etc.
Apparently pipes are also not safe, and even a printf can be an external
process in some environments.

edit: managed to reduce this patch today, so now it's below 60 lines of extra code,
down from ~100.

[jay]

curl -K - << 'EOF'
  verbose
  anyauth
  noproxy *
  user root:thisisthepassword
  url 192.168.1.80:8000/axis-cgi/pwdgrp.cgi
  data action=add&user=operator&pwd=alsoapassword&grp=operator
EOF

Note in this example (which I stole from one of my mailing list posts) the 'EOF' is quoted to prevent variable expansion, sometimes people allow that though. Also note if you do it at the prompt the whole thing will likely go in .bash_history

[vszakats]

curl -K - << 'EOF'
  verbose
  anyauth
  noproxy *
  user root:thisisthepassword
  url 192.168.1.80:8000/axis-cgi/pwdgrp.cgi
  data action=add&user=operator&pwd=alsoapassword&grp=operator
EOF

Note in this example (which I stole from one of my mailing list posts) the 'EOF' is quoted to prevent variable expansion, sometimes people allow that though. Also note if you do it at the prompt the whole thing will likely go in .bash_history

I didn't know about the EOF quoting trick! Though often the secret
comes from somewhere dynamically, e.g. from /usr/bin/security on
macOS, and expansion is desired in such cases. Such redirection is what
I ended up with. Unless there are multiple secrets to pass, in which case
there is the exec 3<<EOF trick for multiple pipes. I forgot which of
these were told to be unsafe, because the pipe appear as a file (?) or
as a /dev/ device somewhere (?). (with curl it's possible to redirect a config
with multiple secrets in them, helping with this.)

One way to avoid redirections is by the app accepting a command, which
it calls to retrieve the secret. That command can be whatever secure
secret retriever method the system provides (e.g. /usr/bin/security or
some secure agent). ssh supports it via SSH_ASKPASS. Also
backup software like Restic and Borg.

[jay]

This is rebuilding the command line in a way that may not accurately represent what it was, aside from the intentional omission of secrets. How and which command line (a&w) is used and how it is parsed varies. I think we may have discussed this at some point in some other issue. Windows doesn't have standard argument parsing and it doesn't give the app the arguments, it just gives it the command line and expects the app to parse it however it chooses.

There's no guarantee that acmd and wcmd will point to the process' internal command line that is used by Windows. Also you are assuming acmdln and wcmdln will both exist and be less than the sum of all the parsed argv plus spaces, which makes sense but afaict isn't guaranteed.

[bagder]

We also have the variable system that is another way to get data into a command line without being visible in the process listing.

I think this solution unfortunately is very complicated, feels brittle and hard to work with due to its completely different nature than the "normal" argument blanking (which value has always been low). This makes me side with @jay here: this implementation does not seem to outweigh the cost of accepting it for the small gain we get with it.

[vszakats]

The UCRT source code says these pointers are initialized with
GetCommandLineW() and GetCommandLineA() before calling
main(). It says they may be NULL. (an earlier iteration of this PR
handled any NULL combinations.)

https://github.com/huangqinjin/ucrt/blob/d6e817a4cc90f6f1fe54f8a0aa4af4fff0bb647d/startup/argv_data.cpp#L63-L64

But, the MSDN documentation says that the buffer returned by
these functions should not be modified, which makes this indeed
brittle, and a strong enough reason to abandon this:
https://learn.microsoft.com/en-us/windows/win32/api/processenv/nf-processenv-getcommandlinew

Return value
The return value is a pointer to the command-line string for the current process.

Remarks
The lifetime of the returned value is managed by the system, applications should not free or modify this value.

Thanks for the reviews and remarks! Closing this without a merge.