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 an issue with the UserProperties structure #281

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cdelafuente-r7
Copy link
Contributor

@cdelafuente-r7 cdelafuente-r7 commented Nov 29, 2024

The last changes of the SAMR DCERPC structures introduced an unexpected bug in the UserProperties structure when instantiating a new object. This PR fixes the issue, but it is still a workaround, there might be a simpler solution.

Without this fix

> pry -r ruby_smb
[1] pry(main)> RubySMB::Dcerpc::Samr::UserProperties.new
<it crashes pry with a giant stacktrace>

With this fix

pry -r ruby_smb
[1] pry(main)> RubySMB::Dcerpc::Samr::UserProperties.new
=> {:reserved1=>0,
 :struct_length=>99,
 :reserved2=>0,
 :reserved3=>0,
 :reserved4=>
  "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
 :property_signature=>80,
 :reserved5=>0}

Scenarios

It has been tested locally with pry to make sure it follows the rules defined in the MS doc:

  • Instantiation and adding/removing UserProperty entries:
[3] pry(main)> ups = RubySMB::Dcerpc::Samr::UserProperties.new
=> {:reserved1=>0,
 :struct_length=>99,
 :reserved2=>0,
 :reserved3=>0,
 :reserved4=>
  "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
 :property_signature=>80,
 :reserved5=>0}
[4] pry(main)> userprop1 = RubySMB::Dcerpc::Samr::UserProperty.new({property_name:"h", property_value:"b"})
=> {:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"b"}
[5] pry(main)> ups.user_properties << userprop1
=> [{:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"b"}]
[6] pry(main)> ups
=> {:reserved1=>0,
 :struct_length=>110,
 :reserved2=>0,
 :reserved3=>0,
 :reserved4=>
  "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
 :property_signature=>80,
 :property_count=>1,
 :user_properties=>[{:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"b"}],
 :reserved5=>0}
[7] pry(main)> ups.user_properties << userprop1
=> [{:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"b"},
 {:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"b"}]
[8] pry(main)> ups
=> {:reserved1=>0,
 :struct_length=>119,
 :reserved2=>0,
 :reserved3=>0,
 :reserved4=>
  "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
 :property_signature=>80,
 :property_count=>2,
 :user_properties=>
  [{:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"b"},
   {:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"b"}],
 :reserved5=>0}
[9] pry(main)> ups.user_properties = []
=> []
[10] pry(main)> ups
=> {:reserved1=>0,
 :struct_length=>99,
 :reserved2=>0,
 :reserved3=>0,
 :reserved4=>
  "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
 :property_signature=>80,
 :reserved5=>0}
  • Reading a binary stream:
[11] pry(main)> ups = RubySMB::Dcerpc::Samr::UserProperties.read("\x00\x00\x00\x00c\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00P\x00\x00")
=> {:reserved1=>0,
 :struct_length=>99,
 :reserved2=>0,
 :reserved3=>0,
 :reserved4=>
  "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
 :property_signature=>80,
 :reserved5=>0}
[12] pry(main)> ups.user_properties << userprop1
=> [{:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"b"}]
[13] pry(main)> ups
=> {:reserved1=>0,
 :struct_length=>110,
 :reserved2=>0,
 :reserved3=>0,
 :reserved4=>
  "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
 :property_signature=>80,
 :property_count=>1,
 :user_properties=>[{:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"b"}],
 :reserved5=>0}
[14] pry(main)> ups = RubySMB::Dcerpc::Samr::UserProperties.read("\x00\x00\x00\x00n\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00P\x00\x01\x00\x02\x00\x01\x00\x00\x00h\x00f\x00")
=> {:reserved1=>0,
 :struct_length=>110,
 :reserved2=>0,
 :reserved3=>0,
 :reserved4=>
  "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
 :property_signature=>80,
 :property_count=>1,
 :user_properties=>[{:name_length=>2, :value_length=>1, :reserved=>0, :property_name=>"h", :property_value=>"f"}],
 :reserved5=>0}

This has also been tested with the Metasploit gather/windows_secrets_dump to make sure it still fixes the original issue.

TODO

Write specs or these new structures.

@cdelafuente-r7 cdelafuente-r7 changed the title Fix issue with the UserProperties structure Fix an issue with the UserProperties structure Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant