Revision 1.202 to ssh/scp.c means to protect against a rogue server that writes to unexpected file names, by checking that the file name returned matches the original glob pattern. The fix has two bugs: 1. If the requested filename contains no / characters, e.g. scp remote:'[xyz]*' . or even scp remote:safefile . no check is done; the remote is permitted to send any file name (hence overwrite any file) it likes. The trouble is that the new code does if ((restrict_pattern = strrchr(src_copy, '/')) != NULL) { *restrict_pattern++ = '\0'; } then, later, if (restrict_pattern != NULL && fnmatch(restrict_pattern, cp, 0) != 0) SCREWUP("filename does not match request"); If there is no /, strrchr returns NULL, restrict_pattern is set to NULL, and fnmatch is not called. One simple fix might be: if ((restrict_pattern = strrchr(src_copy, '/')) != NULL) { *restrict_pattern++ = '\0'; } else restrict_pattern = src_copy; 2. Rather more subtly: before the fix, one could copy several files while connecting and authenticating only once by doing scp remote:'dir1/file1 dir2/file2 dir3/file3' . or even scp remote:'dir1/*.c dir2/*.[ch] dir3/*.o' . Since the fix doesn't account for white space, it would allow only file3 to be copied in the first case, and only file names matching *.o (regardless of which directory they came from) in the second. One can quibble about whether multiple file names should have been allowed like that in the first place, but it has worked for a long time and some* have written scripts that expect it. More to the point, it makes the current fix do the test wrong. Fixing it to preserve the old behaviour seems worthwhile, but adds complexity (I guess you'd need to allocate a table to hold all the patterns, or just split remote names into white-space-separated pieces at a much-higher level in the code). I can see the argument for not doing that, or at least deferring it, since an insistent user can use -T as a workaround. But if the code isn't that white-space-aware, it should at least check for and reject remote names containing white space, since white space breaks the safety check. Thanks, Norman Wilson Computer Science, University of Toronto
Thanks - the changes in rev 1.202 weren't complete, e.g. they don't include support for {foo,bar} alternations. I've committed a fix that also corrected the paths-without-slash problem along the way. I'll take a look at the space-separated path thing; it might be difficult to discern a space-separated source path that refers to multiple files from one that refers to a single file with spaces in its name. It really depends on the server's rules, and the client has no way of knowing these. In practice, /bin/sh is likely to be doing the dequoting/expansion, except when it isn't.
Just to add a bit more to this problem, I've found that using single quotes for the path component also causes it to fail with 'filename does not match request'. For example If I was to do; scp -o User=username "[host]:'/tmp/file.txt'" or even scp -o User=username '[host]:'"'"'/tmp/file.txt'"'"'' It will fail with protocol error: filename does not match request This worked perfectly fine before this patch and I don't see a reason why it shouldn't continue to work going forward. Quotes aren't used all the time but if the path we are trying to fetch has a space it is needed. We also use single quotes so we don't have to escape any metachars, e.g. scp -o User=username '[host]:'"'"'/tmp/file $TERM.txt'"'"'' Would fetch the literal path '/tmp/file $TERM.txt' and not expand $TERM. An alternate way would be to escape paths and meta chars with '\' but that seems to be dependent on the shell used on the remote and potentially fraught with danger. Being able to just enclose the path with a single quote is a lot simpler. https://github.com/ansible/ansible/issues/52640 has some more issues and how we came across this problem.
I honestly don't know how far we're willing to chase this down in scp - these are all extremely fiddly and easy to get wrong in the client. To do it right means basically having to re-implement the entirety of shell quote handling in scp. Even then, it all goes out the window if the server happens to be using a different shell. In any case, it's too late to get fixes for this into OpenSSH 8.0. I suggest adjusting your workflows to use sftp or otherwise using the new scp -T flag to disable the extra client-side checking.
Created attachment 3331 [details] 0001-scp-show-filename-match-patterns-in-verbose-mode.patch With current partial implementation of expansion isn't easy to check why the expression was rejected and the error message isn't much helpful. Would be acceptable to print out on failure the received filename and expected patterns? The attached patch prints that in verbose mode.