Fix SECURITY_DESCRIPTOR computation (#4809)

Author: gpotter2

scapy/layers/smb2.py

@@ -1550,21 +1550,21 @@ class SECURITY_DESCRIPTOR(_NTLMPayloadPacket):
                 "SELF_RELATIVE",
             ],
         ),
-        LEIntField("OwnerSidOffset", 0),
-        LEIntField("GroupSidOffset", 0),
-        LEIntField("SACLOffset", 0),
-        LEIntField("DACLOffset", 0),
+        LEIntField("OwnerSidOffset", None),
+        LEIntField("GroupSidOffset", None),
+        LEIntField("SACLOffset", None),
+        LEIntField("DACLOffset", None),
         _NTLMPayloadField(
             "Data",
             OFFSET,
             [
                 ConditionalField(
                     PacketField("OwnerSid", WINNT_SID(), WINNT_SID),
-                    lambda pkt: pkt.OwnerSidOffset,
+                    lambda pkt: pkt.OwnerSidOffset != 0,
                 ),
                 ConditionalField(
                     PacketField("GroupSid", WINNT_SID(), WINNT_SID),
-                    lambda pkt: pkt.GroupSidOffset,
+                    lambda pkt: pkt.GroupSidOffset != 0,
                 ),
                 ConditionalField(
                     PacketField("SACL", WINNT_ACL(), WINNT_ACL),
@@ -1579,6 +1579,26 @@ class SECURITY_DESCRIPTOR(_NTLMPayloadPacket):
         ),
     ]
 
+    def post_build(self, pkt, pay):
+        # type: (bytes, bytes) -> bytes
+        return (
+            _NTLM_post_build(
+                self,
+                pkt,
+                self.OFFSET,
+                {
+                    "OwnerSid": 4,
+                    "GroupSid": 8,
+                    "SACL": 12,
+                    "DACL": 16,
+                },
+                config=[
+                    ("Offset", _NTLM_ENUM.OFFSET),
+                ]
+            )
+            + pay
+        )
+
 
 # [MS-FSCC] 2.4.2 FileAllInformation
 

scapy/modules/ldaphero.py

@@ -1191,6 +1191,12 @@ def edit(*_):
                 control |= v
         nTSecurityDescriptor.Control = control
 
+        # Offsets need to be recalculated
+        nTSecurityDescriptor.OwnerSidOffset = None
+        nTSecurityDescriptor.GroupSidOffset = None
+        nTSecurityDescriptor.DACLOffset = None
+        nTSecurityDescriptor.SACLOffset = None
+
         # Pfew, we did it. That was some big UI.
 
         # Now update the SD.

test/scapy/layers/smb2.uts

@@ -549,3 +549,29 @@ assert isinstance(set_info.Data, FileRenameInformation)
 assert set_info.Data.FileName == "test"
 assert not set_info.Data.ReplaceIfExists
 
+= SMB2 - Build and dissect SECURITY_DESCRIPTOR
+
+sd = SECURITY_DESCRIPTOR(
+    Control="DACL_PRESENT+DACL_PROTECTED+SELF_RELATIVE",
+    OwnerSid=WINNT_SID.fromstr("S-1-1-0"),
+    GroupSid=WINNT_SID.fromstr("S-1-1-0"),
+    DACL=WINNT_ACL(
+        Aces=[
+            WINNT_ACE_HEADER() / WINNT_ACCESS_ALLOWED_ACE(
+                Mask=1,
+                Sid=WINNT_SID.fromstr("S-1-1-0"),
+            )
+        ]
+    )
+)
+
+sd = SECURITY_DESCRIPTOR(bytes(sd))
+
+assert sd.OwnerSidOffset == 20
+assert sd.GroupSidOffset == 32
+assert sd.SACLOffset == 0
+assert sd.DACLOffset == 44
+
+assert sd.OwnerSid.summary() == "S-1-1-0"
+assert sd.GroupSid.summary() == "S-1-1-0"
+assert sd.DACL.toSDDL() == ['(A;;;;;S-1-1-0)']