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

Fix for vbapplication payload generation #30

Merged
merged 2 commits into from Jul 1, 2021

Conversation

rrockru
Copy link
Contributor

@rrockru rrockru commented Jun 5, 2020

There seems to be a problem with vbapplication payload generation. The loop starts at 1, so the first byte is lost. This fix resolves this issue.

@bcoles
Copy link

bcoles commented Jun 5, 2020

Looks like a bug at first glance. Presumably this code is also affected?

    #
    # Converts a raw string to a vbscript byte array
    #
    def self.to_vbscript(str, name = "buf")
      return "#{name}" if str.nil? or str.empty?

      code = str.unpack('C*')
      buff = "#{name}=Chr(#{code[0]})"
      1.upto(code.length-1) do |byte|
        if(byte % 100 == 0)
          buff << "\r\n#{name}=#{name}"
        end
        # exe is an Array of bytes, not a String, thanks to the unpack
        # above, so the following line is not subject to the different
        # treatments of String#[] between ruby 1.8 and 1.9
        buff << "&Chr(#{code[byte]})"
      end

      return buff
    end

@rrockru
Copy link
Contributor Author

rrockru commented Jun 5, 2020

I think I have found why nobody found this earlier. If this encoder used with msf-generated payload, then almost all the time first byte consists CLD instruction. So removing it was not critical. But on custom payloads it may made payload not usable.

@rrockru
Copy link
Contributor Author

rrockru commented Jun 5, 2020

Looks like a bug at first glance. Presumably this code is also affected?

    #
    # Converts a raw string to a vbscript byte array
    #
    def self.to_vbscript(str, name = "buf")
      return "#{name}" if str.nil? or str.empty?

      code = str.unpack('C*')
      buff = "#{name}=Chr(#{code[0]})"
      1.upto(code.length-1) do |byte|
        if(byte % 100 == 0)
          buff << "\r\n#{name}=#{name}"
        end
        # exe is an Array of bytes, not a String, thanks to the unpack
        # above, so the following line is not subject to the different
        # treatments of String#[] between ruby 1.8 and 1.9
        buff << "&Chr(#{code[byte]})"
      end

      return buff
    end

Nope, this code adds first byte before loop.

@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Jul 1, 2021

Sorry for the delay. Tested manually, although it would be very easy to add tests for this functionality - there's some examples in the repo of how to write tests too if it's something that interests

Before

irb(main):040:0> to_vbapplication('A')
=> "buf = Array()\r\n"
to_vbapplication('ABC')
=> "buf = Array(66,67)\r\n"
puts to_vbapplication('A' * 300); nil
=> buf = Array(65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65, _
65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65, _
65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65, _
65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65)

irb(main):210:0> to_vbapplication('A' * 300).scan('65').count
=> 299

After

to_vbapplication('A')
=> buf = Array(65))
to_vbapplication('ABC')
=> buf = Array(65,66,67)
puts to_vbapplication('A' * 300); nil
=> buf = Array(65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65, _
65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65, _
65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65, _
65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65,65)
to_vb

to_vbapplication('A' * 300).scan('65').count
=> 300

Conclusion

So confirmed - the first character was being dropped, and is now fixed for small + large inputs. Hopefully I didn't miss any edge cases. Thanks for the contribution! 👍

@adfoster-r7 adfoster-r7 merged commit 9f9edca into rapid7:master Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants