Fix an issue with the UserProperties structure

[cdelafuente-r7]

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.

[smcintyre-r7]

Thank you very much for making that change to #display_user_properties?. Everything looks good to me. I tested that an object can be initialized with its default state and not crash. I also tested the original PR, to ensure that issue is still resolved.
rapid7/metasploit-framework#19665

I'm going to go ahead and land this. Thanks @cdelafuente-r7