msys2 / msys2/msys2-runtime

dcrt0.cc: impossible to pass double quotes to application if noglob is enabled

Open
#178 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
240
Forks
60
Avg merge
2d 3h
Merged PRs (30d)
1

Description

Hi.

As of 79a9de58c4f4292ca73c4fb635dca23026eb29bb double quote handling in dcrt0.cc build_argv () is inconsistent when MSYS=noglob is used.

If winshell is set, only backslash characters are skipped. However, noglob causes quoted () to jump to (any) next double quote character. This is not good, as there might be escaped double quotes in the path, which should be skipped when looking for the next (unescaped) double quote.

https://github.com/msys2/msys2-runtime/blob/79a9de58c4f4292ca73c4fb635dca23026eb29bb/winsup/cygwin/dcrt0.cc#L162-L169

build_argv () is also called when launched via strace, which complicates debugging this issue. This issue is also present when new processes are created via CreateProcessA and/or CreateProcessW API in similar circumstances.

noglob is required, if the application would like to use command line arguments longer than 8192 characters and the command line includes any of the following characters: ?*["'(){} (note that both single and double quote characters are included meaning that glob () code is typically hit for shell one-liners). This is because glob () truncates the input, when making Unicode conversion for the pattern. For some reason (perhaps chicked and egg type problem), the buffer is not allocated from heap, but from stack, and this is why the limit is so low.

Nevertheless, if build_argv () is to be called, but glob isn't set, there appears to be no method to pass a double quotes to the application. Consider the following example application:

# cat dump.c
#include <windows.h>
#include <stdio.h>
#include <string.h>
int main (int argc, char ** argv)
  {
  fprintf(stderr, "``%s''.\n", GetCommandLineA());
  for (char ** argp = argv; argp < &argv[argc]; ++argp)
  fprintf(stderr, "\%ld: `%s', length = %lu\n",
      argp - argv, *argp, strlen(*argp));
  return 0;
  }
# gcc -Wall -Wextra dump.c -o dump

When executed inside a typical shell. I presume this works because the shell uses spawn () or fork () and exec (), as unless winshell is set, the quoting behavior is similar to noglob being currently set.

# MSYS=noglob ./dump.exe "\"\\\"hello, world\\\", said quoteception\""
``<...>\dump.exe "\"\\\"hello, world\\\", said quoteception\""''.
0: `./dump', length = 6
1: `"\"hello, world\", said quoteception"', length = 37

So far so good. However, pass thru strace to trigger build_argv (). The same effect can be archieved by executing the application directly from Windows, command prompt etc.

# MSYS=noglob strace ./dump.exe "\"\\\"hello, world\\\", said quoteception\"" |grep arg
   54   24093 [main] dump NNNN build_argv: cmd = '<...>\dump.exe """\""hello, world\"", said quoteception"""', winshell = 1, glob = 0
   64   24157 [main] dump NNNN build_argv: argv[0] = '<...>\dump.exe'
   33   24190 [main] dump NNNN build_argv: argv[1] = '\hello, world\, said quoteception'
   32   24222 [main] dump NNNN build_argv: argc 2
``<...>\dump.exe """\""hello, world\"", said quoteception"""''.
0: `<...>/dump', length = N
1: `\hello, world\, said quoteception', length = 33

The double quotes are now gone, meaning that the application (e.g. shell script) will not likely work as expected.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in winsup/cygwin/dcrt0.cc, especially build_argv() and quoted(), and reproduce the MSYS=noglob case shown with dump.exe through strace. Trace how escaped double quotes are scanned when glob is disabled and compare direct and CreateProcessA/CreateProcessW launches. Done means escaped double quotes reach the application correctly without breaking existing winshell behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.