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

--create-dirs fails when run as system #4796

Closed
mbeifuss opened this issue Jan 7, 2020 · 2 comments
Closed

--create-dirs fails when run as system #4796

mbeifuss opened this issue Jan 7, 2020 · 2 comments

Comments

@mbeifuss
Copy link

@mbeifuss mbeifuss commented Jan 7, 2020

Commit f16bed0 broke the ability to use the --create-dirs to create folders from a service running as system (not sure if that is a unique case or not). The service calls curl The command below fails with this change, reverting the change fixes the issue.
"C:\Program Files (x86)\curl\curl.exe" -k -L -o "D:\temp\system\curl.exe.tmp" -D "D:\temp\system\curl.exe.hdr" --create-dirs https://10.10.1.1/Download/curl.exe, creates the folder but then errors out and fails to download the file(s) this is whether the folder(s) existed or not.
We are calling curl from the service via CreateProcess(exeName, argList, &saProc, &saThread,
FALSE, CREATE_BREAKAWAY_FROM_JOB, NULL, NULL, &si, &piProc);

I did this

"C:\Program Files (x86)\curl\curl.exe" -k -L -o "D:\temp\system\curl.exe.tmp" -D "D:\temp\system\curl.exe.hdr" --create-dirs https://10.10.1.1/Download/curl.exe

I expected the following

folder temp\system to be created and curl.exe to be downloaded into it. Folder does get created but then it fails to download the file and errors with the message You don't have permission to create D:.

curl/libcurl version

7.64.0
[curl -V output]
curl 7.64.0 (i386-pc-win32) libcurl/7.64.0 OpenSSL/1.1.1a (Schannel) zlib/1.2.11 brotli/1.0.7 WinIDN libssh2/1.8.0 nghttp2/1.36.0
Release-Date: 2019-02-06
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IDN IPv6 Largefile SSPI Kerberos SPNEGO NTLM SSL libz brotli TLS-SRP HTTP2 HTTPS-proxy MultiSSL

operating system

Windows 10

@bagder bagder added the cmdline tool label Jan 7, 2020
@bagder bagder changed the title Commit f16bed0c45dc63864fe2097b7df939276d96d62b breaks --create-dirs when run as system --create-dirs fails when run as system Jan 7, 2020
@bagder bagder added the Windows label Jan 7, 2020
@jay

This comment has been minimized.

Copy link
Member

@jay jay commented Jan 7, 2020

Confirmed, it's a directory traversal issue. I'm on it.

jay referenced this issue Jan 7, 2020
Patch-by: Jay Satiro
Detected by Coverity
Fixes #2739
Closes #2912
jay added a commit to jay/curl that referenced this issue Jan 8, 2020
- When creating a directory hierarchy do not error when mkdir fails due
  to error EACCESS (13) "access denied".

Some file systems allow for directory traversal; in this case that it
should be possible to create child directories when permission to the
parent directory is restricted.

This is a regression caused by me in f16bed0 (precedes curl-7_61_1).
Basically I had assumed that if a directory already existed it would
fail only with error EEXIST, and not error EACCES. The latter may
happen if the directory exists but has certain restricted permissions.

Reported-by: mbeifuss@users.noreply.github.com

Fixes curl#4796
Closes #xxxx
@jay

This comment has been minimized.

Copy link
Member

@jay jay commented Jan 8, 2020

Please try #4797.

create_dir_hierarchy erroneously attempts to create the drive's current directory which may not exist, and if it does exist may be access denied. This is specific to Windows/MSDOS which stores a per-process table of drives and their current directories. For example your outfile D:\temp\system\curl.exe.tmp is broken down and mkdir called on each token:

dirbuildup: D:
dirbuildup: D:\temp
dirbuildup: D:\temp\system

D: represents the current directory so it's calling mkdir D: which could be D:\foo\bar or whatever for all we know. Although in this case we'll assume it's actually D:\ which would cause access denied. Prior versions of curl work because they ignored access denied errors and allowed for directory traversal, which is acceptable with the outfile path. The changes I made restore that behavior (without the race condition whose fix caused this regression) and also I added a check for current directory drives to skip them. In other words no longer mkdir D: there's no reason for that.

@jay jay closed this in 4027bd7 Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.