Skip to content

Problem with single quotes in run_wql #143

Open
Open
Problem with single quotes in run_wql#143
Labels
bug

Description

@Mazwak
Mazwak
opened on May 22, 2015

Hi

I was trying to enumerate directories with run_wql, and winRM crashes if the directory has a quote in his name.

Here is the query :
ASSOCIATORS OF {Win32_Directory.Name="#{path}"} WHERE AssocClass = Win32_Subdirectory ResultRole = PartComponent

Applying a similar fix as #73 in run_wql seems to fix the problem.

Activity

  1. Mazwak
    changed the title [-]Problem with quotes in run_wql[/-] [+]Problem with single quotes in run_wql[/+] on May 22, 2015

  2. Mazwak commented on May 25, 2015

    @Mazwak
    Mazwak
    on May 25, 2015
    ContributorAuthor

    Here is a patch for the 2 issues, which is more concise :

    diff --git a/lib/winrm/winrm_service.rb b/lib/winrm/winrm_service.rb
index 0882832..e7d1345 100644
--- a/lib/winrm/winrm_service.rb
+++ b/lib/winrm/winrm_service.rb
@@ -157,16 +157,11 @@ module WinRM
       builder.tag! :env, :Envelope, namespaces do |env|
         env.tag!(:env, :Header) { |h| h << Gyoku.xml(merge_headers(header,resource_uri_cmd,action_command,h_opts,selector_shell_id(shell_id))) }
         env.tag!(:env, :Body) do |env_body|
-          env_body.tag!("#{NS_WIN_SHELL}:CommandLine") { |cl| cl << Gyoku.xml(body) }
+          env_body.tag!("#{NS_WIN_SHELL}:CommandLine") { |cl| cl << Gyoku.xml(body).gsub('&#39;', '\'') }
         end
       end

-      # Grab the command element and unescape any single quotes - issue 69
-      xml = builder.target!
-      escaped_cmd = /<#{NS_WIN_SHELL}:Command>(.+)<\/#{NS_WIN_SHELL}:Command>/m.match(xml)[1]
-      xml[escaped_cmd] = escaped_cmd.gsub(/&#39;/, "'")
-
-      resp_doc = send_message(xml)
+      resp_doc = send_message(builder.target!)
       command_id = REXML::XPath.first(resp_doc, "//#{NS_WIN_SHELL}:CommandId").text

       if block_given?
@@ -336,14 +331,14 @@ module WinRM
       builder.tag! :env, :Envelope, namespaces do |env|
         env.tag!(:env, :Header) { |h| h << Gyoku.xml(merge_headers(header,resource_uri_wmi,action_enumerate)) }
         env.tag!(:env, :Body) do |env_body|
-          env_body.tag!("#{NS_ENUM}:Enumerate") { |en| en << Gyoku.xml(body) }
+          env_body.tag!("#{NS_ENUM}:Enumerate") { |en| en << Gyoku.xml(body).gsub('&#39;', '\'') }
         end 
       end

    I didn't test extensively, but it should work.

  3. sneal commented on May 27, 2015

    @sneal
    sneal
    on May 27, 2015
    Member

    Thanks, this LGTM! I wish there was a comment around the old implementation, because it is more complex than just a gsub. I suspect I was just being conservative.

  4. Mazwak commented on May 28, 2015

    @Mazwak
    Mazwak
    on May 28, 2015
    ContributorAuthor

    As far as I understand, you were matching the command part of the commandline, and unescaping the quotes.
    It works as long as the command has no quotes in arguments, but fails otherwise.

    You can test with :

    puts wws.cmd %Q|echo Hello Bob's world|
puts wws.cmd 'echo', ["Hello", "Bob's", "world"]

    The second one fails with current code, and works with mine.
    I don't know why you excluded the arguments in your fist version.

    PS: what is "LGTM" ? Looks good to me ?

  5. Mazwak commented on May 28, 2015

    @Mazwak
    Mazwak
    on May 28, 2015
    ContributorAuthor

    While testing all this, I noticed another problem. You accept commands either as a big string, or as a string and an array of arguments.
    I think the command as a big string is a bad idea. Right now, it makes the command + arguments just fail.
    I would just raise an error if the command has spaces in it. However, you might just want to issue a warning, and split it. Here is what I came to, almost verbatim from shellwords :

        def shell_split(line)
      words = []
      field = ''
      line.gsub!(/\^(['"])/, '\\\\\1')
      line.scan(/\G\s*(?>([^\s\\\'\"]+)|'([^\']*)'|"((?:[^\"\\]|\\.)*)"|(\\.?)|(\S))(\s|\z)?/m) do
        |word, sq, dq, esc, garbage, sep|
        raise ArgumentError, "Unmatched double quote: #{line.inspect}" if garbage
        field << (word || sq || (dq || esc).gsub(/\\(.)/, '\\1'))
        if sep 
          words << field.gsub(/(['"])/, '^\1')
          field = ''
        end 
      end 
      words
    end

    I just change "^'" and "^"" before parsing, and change them back after. And in run_command :

            command, *arguments = shell_split(command).concat(arguments)
        body = { "#{NS_WIN_SHELL}:Command" => "#{command}", "#{NS_WIN_SHELL}:Arguments" => arguments}

    Ideally, run_command should raise an error, and run_cmd should issue a warning, before splitting the command. All applied, it looks like this :
    https://gist.github.com/Mazwak/19f1176a95c0b10f9a89

  6. sneal commented on Jun 6, 2015

    @sneal
    sneal
    on Jun 6, 2015
    Member

    It looks like I need to look into this a bit more and see what the actual behavior is. From the MS documentation it looks like you can pass arguments either way either via the command line or via arguments, but not both.

    commandLine
    Defines a required null-terminated string that represents the command to be executed. Typically, the command is specified without any arguments, which are specified separately. However, a user can specify the command line and all of the arguments by using this parameter. If arguments are specified for the commandLine parameter, the args parameter should be NULL.

  7. sneal
    added
    bug
    on Nov 6, 2015

  8. Mazwak commented on Feb 18, 2016

    @Mazwak
    Mazwak
    on Feb 18, 2016
    ContributorAuthor

    I have a new duplicate problem. It seems now Gyoku changes quotes to &quot; instead of &#39;.

    Forget about the arguments issue, I will open a new one. However, I would like the quote issue to be fixed. As I did not find how to tell Gyoku not to escape quotes, I just gsubed them back :

    diff --git a/lib/winrm/winrm_service.rb b/lib/winrm/winrm_service.rb
index 8ce4fc8..3ad8261 100644
--- a/lib/winrm/winrm_service.rb
+++ b/lib/winrm/winrm_service.rb
@@ -392,7 +392,7 @@ module WinRM
       builder.tag! :env, :Envelope, namespaces do |env|
         env.tag!(:env, :Header) { |h| h << Gyoku.xml(merge_headers(header,resource_uri_wmi,action_enumerate)) }
         env.tag!(:env, :Body) do |env_body|
-          env_body.tag!("#{NS_ENUM}:Enumerate") { |en| en << Gyoku.xml(body) }
+          env_body.tag!("#{NS_ENUM}:Enumerate") { |en| en << Gyoku.xml(body).gsub('&quot;', "'") }
         end
       end

  9. Mazwak commented on Feb 18, 2016

    @Mazwak
    Mazwak
    on Feb 18, 2016
    ContributorAuthor

    I just read #131 and I don't have time right now for a better patch.
    I should work on it soon though.

  10. Mazwak commented on Feb 19, 2016

    @Mazwak
    Mazwak
    on Feb 19, 2016
    ContributorAuthor

    After looking more carefully, I think I have a better patch. Instead of bruteforcing Gyoku output, it seems possible to make it not change the input.

    I did not test run_cmd, but I did test run_wql.

    diff --git a/lib/winrm/winrm_service.rb b/lib/winrm/winrm_service.rb
index 8ce4fc8..f643445 100644
--- a/lib/winrm/winrm_service.rb
+++ b/lib/winrm/winrm_service.rb
@@ -169,9 +169,13 @@ module WinRM
       consolemode = cmd_opts.has_key?(:console_mode_stdin) ? cmd_opts[:console_mode_stdin] : 'TRUE'
       skipcmd     = cmd_opts.has_key?(:skip_cmd_shell) ? cmd_opts[:skip_cmd_shell] : 'FALSE'

-      h_opts = { "#{NS_WSMAN_DMTF}:OptionSet" => {
-        "#{NS_WSMAN_DMTF}:Option" => [consolemode, skipcmd],
-        :attributes! => {"#{NS_WSMAN_DMTF}:Option" => {'Name' => ['WINRS_CONSOLEMODE_STDIN','WINRS_SKIP_CMD_SHELL']}}}
+      h_opts = {
+        "#{NS_WSMAN_DMTF}:OptionSet" => {
+          "#{NS_WSMAN_DMTF}:Option" => [
+            { :@Name => 'WINRS_CONSOLEMODE_STDIN', :content! => consolemode },
+            { :@Name => 'WINRS_SKIP_CMD_SHELL', :content! => skipcmd }
+          ]
+        }
       }
       body = { "#{NS_WIN_SHELL}:Command" => "\"#{command}\"", "#{NS_WIN_SHELL}:Arguments" => arguments}

@@ -184,12 +188,7 @@ module WinRM
         end
       end

-      # Grab the command element and unescape any single quotes - issue 69
-      xml = builder.target!
-      escaped_cmd = /<#{NS_WIN_SHELL}:Command>(.+)<\/#{NS_WIN_SHELL}:Command>/m.match(xml)[1]
-      xml[escaped_cmd] = escaped_cmd.gsub(/&#39;/, "'")
-
-      resp_doc = send_message(xml)
+      resp_doc = send_message(builder.target!)
       command_id = REXML::XPath.first(resp_doc, "//#{NS_WIN_SHELL}:CommandId").text

       if block_given?
@@ -382,9 +381,11 @@ module WinRM
     def run_wql(wql)

       body = { "#{NS_WSMAN_DMTF}:OptimizeEnumeration" => nil,
-        "#{NS_WSMAN_DMTF}:MaxElements" => '32000',
-        "#{NS_WSMAN_DMTF}:Filter" => wql,
-        :attributes! => { "#{NS_WSMAN_DMTF}:Filter" => {'Dialect' => 'http://schemas.microsoft.com/wbem/wsman/1/WQL'}}
+               "#{NS_WSMAN_DMTF}:MaxElements" => '32000',
+               "#{NS_WSMAN_DMTF}:Filter" => [
+                 { :@Dialect => 'http://schemas.microsoft.com/wbem/wsman/1/WQL',
+                   :content! => wql }
+               ]
       }

       builder = Builder::XmlMarkup.new
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bug

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions