Fix bash completion issue

Reported by Stephane Chazelas. Potentially sensitive arguments could be leaked to the environment during completion computation.

Fix bash completion issue
Reported by Stephane Chazelas. Potentially sensitive arguments could be leaked to the environment during completion computation.
Jay Berkenbilt
1 parent 47a88683
Showing 9 changed files with 72 additions and 30 deletions
completions/bash/qpdf
completions/zsh/_qpdf
generate_auto_job
job.sums
libqpdf/QPDFArgParser.cc
libqpdf/qpdf/auto_job_help.hh
manual/cli.rst
manual/qpdf.1
manual/release-notes.rst
-eval $(/usr/bin/qpdf --completion-bash)
+eval "$(/usr/bin/qpdf --completion-bash)"
 #compdef qpdf
-eval $(/usr/bin/qpdf --completion-zsh)
+eval "$(/usr/bin/qpdf --completion-zsh)"
@@ -351,6 +351,16 @@ class Main:
             for k, v in hashes.items():
                 print(f'{k} {v}', file=f)
+    @staticmethod
+    def quotes(long_text):
+        if '"(' in long_text or ')"' in long_text:
+            r_quote_open = 'R"/('
+            r_quote_close = ')/"'
+        else:
+            r_quote_open = 'R"('
+            r_quote_close = ')"'
+        return (r_quote_open, r_quote_close)
+
     def generate_doc(self, df, f, f_man):
         """
         Generates documentation and help-related functionalities for a given parser.
@@ -498,8 +508,10 @@ class Main:
             elif state == st_topic:
                 if append_long_text(line, topic):
                     self.all_topics.add(topic)
+                    r_quote_open, r_quote_close = self.quotes(long_text)
                     print(f'ap.addHelpTopic("{topic}", "{short_text}",'
-                          f' R"({long_text})");', file=f)
+                          f' {r_quote_open}{long_text}{r_quote_close});',
+                          file=f)
                     print(f'.SH {topic.upper()} ({short_text})', file=f_man)
                     print(manify(long_text), file=f_man, end='')
                     help_lines += 1
@@ -523,8 +535,11 @@ class Main:
                             f' lineno={lineno}')
                     if option not in self.help_options:
                         self.jdata[option[2:]]['help'] = short_text
+                    r_quote_open, r_quote_close = self.quotes(long_text)
                     print(f'ap.addOptionHelp("{option}", "{topic}",'
-                          f' "{short_text}", R"({long_text})");', file=f)
+                          f' "{short_text}",'
+                          f' {r_quote_open}{long_text}{r_quote_close});',
+                          file=f)
                     if last_option_topic != topic:
                         print('.PP\nRelated Options:', file=f_man)
                     last_option_topic = topic
 # Generated by generate_auto_job
 CMakeLists.txt 18214e276670dc8beb2ab83f789c6d94941bc92b199b353f3943024cfd41d3bc
-generate_auto_job 280b75d5307c537385a75ec588493496cfb0bc754d48c34ca8c42bbc55dd717b
+generate_auto_job 8e3175a515aa8837d8a01bba0346b04b3d777d70330ba5b7d52f691316054a34
 include/qpdf/auto_job_c_att.hh 4c2b171ea00531db54720bf49a43f8b34481586ae7fb6cbf225099ee42bc5bb4
 include/qpdf/auto_job_c_copy_att.hh 50609012bff14fd82f0649185940d617d05d530cdc522185c7f3920a561ccb42
 include/qpdf/auto_job_c_enc.hh 28446f3c32153a52afa239ea40503e6cc8ac2c026813526a349e0cd4ae17ddd5
@@ -9,12 +9,12 @@ include/qpdf/auto_job_c_pages.hh 09ca15649cc94fdaf6d9bdae28a20723f2a66616bf15aa8
 include/qpdf/auto_job_c_uo.hh 9c2f98a355858dd54d0bba444b73177a59c9e56833e02fa6406f429c07f39e62
 job.yml e136726a6c7e43736b1f75f4de347fa50baf5f38ed1da58647ce2e980751fb29
 libqpdf/qpdf/auto_job_decl.hh 34ba07d3891c3e5cdd8712f991e508a0652c9db314c5d5bcdf4421b76e6f6e01
-libqpdf/qpdf/auto_job_help.hh bcfe600cc01447a7fae8661a8d101c7b15ce144bb06ba0beab656f3a655371d1
+libqpdf/qpdf/auto_job_help.hh d0cca031e99f10caa3f4b70ea574b36b0af63d24de333e7d6f0bf835e959f0be
 libqpdf/qpdf/auto_job_init.hh 02c526c37ad4051cac956ac7c12ae1d020517264f3f3d3beabb066ae2529e4bf
 libqpdf/qpdf/auto_job_json_decl.hh 04965f6321e54b8b3b1dd2ca101d763a22ab44fa81c69e4b6fc0fd6bb7f50f92
 libqpdf/qpdf/auto_job_json_init.hh b49378f00d521a9f3e0ce9086e30b082bc6ef8e43c845e2a3c99857b72448307
 libqpdf/qpdf/auto_job_schema.hh f6a3e8b663714bba50b594f5e31437bbcb96ca4609d2c150c3bbc172e3b000fa
 manual/_ext/qpdf.py 6add6321666031d55ed4aedf7c00e5662bba856dfcd66ccb526563bffefbb580
-manual/cli.rst 6fae28c9589bfde5b55260c95a7c64ad48688875f14f195129606405b32a04c6
-manual/qpdf.1 358dfe1bbeb49366d6dd17f74d883295344725b985183b8ca5e23226461654b3
+manual/cli.rst b7bd5e34495d3f9156ff6242988dba73a2e5dce33d71f75ec1415514a3843f35
+manual/qpdf.1 d5785d23e77b02a77180419d87787002dc244d82d586d56008ab603299f565fd
 manual/qpdf.1.in 436ecc85d45c4c9e2dbd1725fb7f0177fb627179469f114561adf3cb6cbb677b
@@ -183,13 +183,27 @@ QPDFArgParser::completionCommon(bool zsh)
         }
     }
     if (zsh) {
-        std::cout << "autoload -U +X bashcompinit && bashcompinit && ";
-    }
-    std::cout << "complete -o bashdefault -o default";
-    if (!zsh) {
-        std::cout << " -o nospace";
+        // FIXME: we assume progname doesn't contain single quote
+        // characters. 's in progname like in "/opt/joe's software/qpdf"
+        // should ideally be escaped as '\''. Unlikely to be a problem
+        // in practice. '...' is preferable over "..." as inside the
+        // latter more characters ("$`\) are a problem and it's
+        // virtually impossible to escape those in a locale-independent
+        // way.
+        std::cout << "autoload -U +X bashcompinit && bashcompinit &&"
+                  << "complete -o bashdefault -o default -C '" << progname << "' " << m->whoami
+                  << "\n";
+    } else {
+        // we need a function wrapper that discards arguments to avoid
+        // leaking sensitive information in the process argument list
+        // which is public on most systems. Here putting the code on one
+        // line as old versions of the documentation were instructing
+        // users to do eval $(qpdf --completion-bash) instead of the
+        // correct eval "$(qpdf --completion-bash)"
+        std::cout << "qpdf_completer() { '" << progname << "'; }; "
+                  << "complete -o bashdefault -o default -o nospace -F qpdf_completer " << m->whoami
+                  << "\n";
     }
-    std::cout << " -C \"" << progname << "\" " << m->whoami << '\n';
     // Put output before error so calling from zsh works properly
     std::string path = progname;
     size_t slash = path.find('/');
@@ -376,9 +390,19 @@ QPDFArgParser::checkCompletion()
         if (p > m->bash_line.length()) {
             p = m->bash_line.length();
         }
-        // Set bash_cur and bash_prev based on bash_line rather than relying on argv. This enables
-        // us to use bashcompinit to get completion in zsh too since bashcompinit sets COMP_LINE and
-        // COMP_POINT but doesn't invoke the command with options like bash does.
+        // Set bash_cur and bash_prev based on bash_line rather than relying on
+        // argv. Using argv is unsafe as process argument lists are public on
+        // most systems. zsh doesn't pass information there, and we actively
+        // discard them for bash with our qpdf_completer to avoid that
+        // information disclosure vulnerability. Both bash and zsh set
+        // COMP_LINE and COMP_POINT which we can rely on instead.
+        //
+        // FIXME. For both bash and zsh, COMP_POINT is an offset in terms of
+        // *characters*, with characters decoded as per the shell's own locale.
+        // Here we interpret it as a *byte* offset which means it won't work
+        // properly if there are multibyte characters to the left of the
+        // cursor, but saves us having to decode the command line (which is hard
+        // to do in the same way the shell does in all cases).
         // p is equal to length of the string. Walk backwards looking for the first separator.
         // bash_cur is everything after the last separator, possibly empty.
@@ -45,11 +45,11 @@ ap.addHelpTopic(&quot;exit-status&quot;, &quot;meanings of qpdf&#39;s exit codes&quot;, R&quot;(Meaning of ex
 ap.addOptionHelp("--warning-exit-0", "exit-status", "exit 0 even with warnings", R"(Use exit status 0 instead of 3 when warnings are present. When
 combined with --no-warn, warnings are completely ignored.
 )");
-ap.addHelpTopic("completion", "shell completion", R"(Shell completion is supported with bash and zsh. Use
-eval $(qpdf --completion-bash) or eval $(qpdf --completion-zsh)
+ap.addHelpTopic("completion", "shell completion", R"/(Shell completion is supported with bash and zsh. Use
+eval "$(qpdf --completion-bash)" or eval "$(qpdf --completion-zsh)"
 to enable. The QPDF_EXECUTABLE environment variable overrides the
 path to qpdf that these commands output.
-)");
+)/");
 ap.addOptionHelp("--completion-bash", "completion", "enable bash completion", R"(Output a command that enables bash completion
 )");
 ap.addOptionHelp("--completion-zsh", "completion", "enable zsh completion", R"(Output a command that enables zsh completion
@@ -244,14 +244,14 @@ Shell Completion
 .. help-topic completion: shell completion
    Shell completion is supported with bash and zsh. Use
-   eval $(qpdf --completion-bash) or eval $(qpdf --completion-zsh)
+   eval "$(qpdf --completion-bash)" or eval "$(qpdf --completion-zsh)"
    to enable. The QPDF_EXECUTABLE environment variable overrides the
    path to qpdf that these commands output.
 :command:`qpdf` provides its own completion support for zsh and bash.
-You can enable bash completion with :command:`eval $(qpdf
---completion-bash)` and zsh completion with :command:`eval $(qpdf
---completion-zsh)`. If :command:`qpdf` is not in your path, you should
+You can enable bash completion with :command:`eval "$(qpdf
+--completion-bash)"` and zsh completion with :command:`eval "$(qpdf
+--completion-zsh)"`. If :command:`qpdf` is not in your path, you should
 use an absolute path to qpdf in the above invocation. If you invoke it
 with a relative path, it will warn you, and the completion won't work
 if you're in a different directory.
@@ -78,7 +78,7 @@ Use exit status 0 instead of 3 when warnings are present. When
 combined with --no-warn, warnings are completely ignored.
 .SH COMPLETION (shell completion)
 Shell completion is supported with bash and zsh. Use
-eval $(qpdf --completion-bash) or eval $(qpdf --completion-zsh)
+eval "$(qpdf --completion-bash)" or eval "$(qpdf --completion-zsh)"
 to enable. The QPDF_EXECUTABLE environment variable overrides the
 path to qpdf that these commands output.
 .PP
@@ -55,12 +55,15 @@ more detail.
   - CLI Enhancements
-   - Disallow option :qpdf:ref:`--deterministic-id` to be used together
-     with the incompatible options :qpdf:ref:`--encrypt` or
-     :qpdf:ref:`--copy-encryption`.
+    - Disallow option :qpdf:ref:`--deterministic-id` to be used together
+      with the incompatible options :qpdf:ref:`--encrypt` or
+      :qpdf:ref:`--copy-encryption`.
-   - Option :qpdf:ref:`--check` now includes additional basic checks of the
-     AcroForm, Dests, Outlines, and PageLabels structures.
+    - Option :qpdf:ref:`--check` now includes additional basic checks of the
+      AcroForm, Dests, Outlines, and PageLabels structures.
+
+    - Fix completion scripts and handling to avoid leaking arguments
+      into the environment during completion.
   - Other enhancements