Merge pull request #70 from Yamato-Security/66-apply-upstream-pr-242-…

…and-245 Apply upstream PR-242/245 (JSON Output improvement)
Yamato-Security · Nov 4, 2024 · 239be92 · 239be92
2 parents c8ed9d2 + cc5b6c3
commit 239be92
Show file tree

Hide file tree

Showing 14 changed files with 422 additions and 363 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog
 
+## [0.8.12 - 2024-11-04]
+
+- JSON output order is now preserved. (omerbenamram/evtx#242)
+- JSON now outputs fields that have the same name multiple times. Before, only the last one was outputted. (omerbenamram/evtx#245)
+
 ## [0.8.11 - 2024-10-30]
 
 Fixed a compiler bug with `quick-xml` 0.37.0. Updated crates. (#67) (@fukusuket)

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -6,7 +6,7 @@ repository = "https://github.com/Yamato-Security/hayabusa-evtx"
 license = "MIT"
 readme = "README.md"
 
-version = "0.8.11"
+version = "0.8.12"
 authors = ["Omer Ben-Amram <[email protected]>, Yamato Security"]
 edition = "2021"
 
@@ -34,7 +34,7 @@ clap = { version = "4", optional = true }
 dialoguer = { version = "*", optional = true }
 indoc = { version = "*", optional = true }
 
-serde_json = "1"
+serde_json = { version = "1", features = ["preserve_order"]}
 
 [target.'cfg(not(windows))'.dependencies]
 # jemalloc is significantly more peformant than the system allocator.

diff --git a/src/json_output.rs b/src/json_output.rs
@@ -214,28 +214,71 @@ impl JsonOutput {
                             .to_string(),
                 }
                 })?;
+                // We do a linear probe in case XML contains duplicate keys
+                if let Some(old_attribute) =
+                    value.insert(format!("{}_attributes", name), Value::Null)
+                {
+                    if let Some(old_value) = value.insert(name.to_string(), Value::Null) {
+                        let mut free_slot = 1;
+                        // If it is a concrete value, we look for another slot.
+                        while value.get(&format!("{}_{}", name, free_slot)).is_some()
+                            || value
+                                .get(&format!("{}_{}_attributes", name, free_slot))
+                                .is_some()
+                        {
+                            // Value is an empty object - we can override it's value.
+                            free_slot += 1
+                        }
+                        if let Some(old_value_object) = old_value.as_object() {
+                            if !old_value_object.is_empty() {
+                                value.insert(format!("{}_{}", name, free_slot), old_value);
+                            }
+                        };
+                        if let Some(old_attribute_object) = old_attribute.as_object() {
+                            if !old_attribute_object.is_empty() {
+                                value.insert(
+                                    format!("{}_{}_attributes", name, free_slot),
+                                    old_attribute,
+                                );
+                            };
+                        };
+                    };
+                };
 
                 value.insert(format!("{name}_attributes"), Value::Object(attributes));
 
                 // If the element's main value is empty, we want to remove it because we
                 // do not want the value to represent an empty object.
-                if value[name] == Value::Object(Map::new()) {
+                if value[name].is_null() || value[name] == Value::Object(Map::new()) {
                     value.remove(name);
                 }
             } else {
-                let value = self
-                    .get_or_create_current_path()
-                    .as_object_mut()
-                    .ok_or_else(|| {
-                        SerializationError::JsonStructureError {
-                    message:
-                        "This is a bug - expected current value to exist, and to be an object type.
-                            Check that the value is not `Value::null`"
-                            .to_string(),
-                }
-                    })?;
+                let container = self.get_current_parent().as_object_mut().ok_or_else(|| {
+                    SerializationError::JsonStructureError {
+                        message:
+                            "This is a bug - expected parent container to exist, and to be an object type.\
+                                Check that the referencing parent is not `Value::null`"
+                                .to_string(),
+                    }
+                })?;
+                // We do a linear probe in case XML contains duplicate keys
+                if let Some(old_value) = container.insert(name.to_string(), Value::Null) {
+                    if let Some(map) = old_value.as_object() {
+                        if !map.is_empty() {
+                            let mut free_slot = 1;
+                            // If it is a concrete value, we look for another slot.
+                            while container.get(&format!("{}_{}", name, free_slot)).is_some() {
+                                // Value is an empty object - we can override it's value.
+                                free_slot += 1
+                            }
+                            container.insert(format!("{}_{}", name, free_slot), old_value);
+                        }
+                    }
+                };
 
+                let mut value = Map::new();
                 value.insert("#attributes".to_owned(), Value::Object(attributes));
+                container.insert(name.to_string(), Value::Object(value));
             }
         } else {
             // If the object does not have attributes, replace it with a null placeholder,
@@ -534,10 +577,10 @@ mod tests {
         let s2 = r#"
 {
   "HTTPResponseHeadersInfo": {
-    "Header": "HTTP/1.1 200 OK",
     "Header_attributes": {
       "attribute1": "NoProxy"
-    }
+    },
+    "Header": "HTTP/1.1 200 OK"
   }
 }
 "#

diff --git a/tests/snapshots/test_record_samples__event_json_missing_string_cache_entry.snap b/tests/snapshots/test_record_samples__event_json_missing_string_cache_entry.snap
@@ -3,34 +3,34 @@ source: tests/test_record_samples.rs
 expression: "&value"
 ---
 {
+  "Event_attributes": {
+    "xmlns": "http://schemas.microsoft.com/win/2004/08/events/event"
+  },
   "Event": {
-    "EventData": null,
     "System": {
-      "Channel": "Security",
-      "Computer": "37L4247F27-25",
-      "Correlation": null,
-      "EventID": 4608,
-      "EventRecordID": 1,
-      "Execution_attributes": {
-        "ProcessID": 456,
-        "ThreadID": 460
-      },
-      "Keywords": "0x8020000000000000",
-      "Level": 0,
-      "Opcode": 0,
       "Provider_attributes": {
-        "Guid": "54849625-5478-4994-A5BA-3E3B0328C30D",
-        "Name": "Microsoft-Windows-Security-Auditing"
+        "Name": "Microsoft-Windows-Security-Auditing",
+        "Guid": "54849625-5478-4994-A5BA-3E3B0328C30D"
       },
-      "Security": null,
+      "EventID": 4608,
+      "Version": 0,
+      "Level": 0,
       "Task": 12288,
+      "Opcode": 0,
+      "Keywords": "0x8020000000000000",
       "TimeCreated_attributes": {
         "SystemTime": "2016-07-08T18:12:51.681640Z"
       },
-      "Version": 0
-    }
-  },
-  "Event_attributes": {
-    "xmlns": "http://schemas.microsoft.com/win/2004/08/events/event"
+      "EventRecordID": 1,
+      "Correlation": null,
+      "Execution_attributes": {
+        "ProcessID": 456,
+        "ThreadID": 460
+      },
+      "Channel": "Security",
+      "Computer": "37L4247F27-25",
+      "Security": null
+    },
+    "EventData": null
   }
 }
diff --git a/tests/snapshots/test_record_samples__event_json_multiple_empty_data_nodes_not_ignored.snap b/tests/snapshots/test_record_samples__event_json_multiple_empty_data_nodes_not_ignored.snap
@@ -1,34 +1,33 @@
 ---
 source: tests/test_record_samples.rs
 expression: "&value"
-
 ---
 {
+  "Event_attributes": {
+    "xmlns": "http://schemas.microsoft.com/win/2004/08/events/event"
+  },
   "Event": {
-    "EventData": {
-      "Data": "Set-Mailbox-Identity \"Administrateur\" -DeliverToMailboxAndForward \"False\" -ForwardingSmtpAddress \"smtp:[email protected]\"ave.local/Users/AdministrateurS-1-5-21-186559946-3925841745-111227986-500S-1-5-21-186559946-3925841745-111227986-500Remote-ManagementShell-Unknown5668 w3wp#MSExchangePowerShellAppPool500:00:26.0389557Afficher la forêt entière : 'False', Portée par défaut : « ave.local », Configuration du contrôleur de domaine : « DC.ave.local », Catalogue global préféré : « DC.ave.local », Contrôleurs de domaine préférés : « { DC.ave.local } »False0 objects execution has been proxied to remote server.0ActivityId: a3591746-a27b-447a-b8be-ff54ae3a46f1ServicePlan:;IsAdmin:True;fr-FR"
-    },
     "System": {
-      "Channel": "MSExchange Management",
-      "Computer": "WEC.ave.local",
-      "EventID": "1",
+      "Provider_attributes": {
+        "Name": "MSExchange CmdletLogs"
+      },
       "EventID_attributes": {
         "Qualifiers": "16384"
       },
-      "EventRecordID": "3229",
-      "Keywords": "0x80000000000000",
+      "EventID": "1",
       "Level": "4",
-      "Provider_attributes": {
-        "Name": "MSExchange CmdletLogs"
-      },
-      "Security": null,
       "Task": "1",
+      "Keywords": "0x80000000000000",
       "TimeCreated_attributes": {
         "SystemTime": "2021-11-19T16:52:33.833733500Z"
-      }
+      },
+      "EventRecordID": "3229",
+      "Channel": "MSExchange Management",
+      "Computer": "WEC.ave.local",
+      "Security": null
+    },
+    "EventData": {
+      "Data": "Set-Mailbox-Identity \"Administrateur\" -DeliverToMailboxAndForward \"False\" -ForwardingSmtpAddress \"smtp:[email protected]\"ave.local/Users/AdministrateurS-1-5-21-186559946-3925841745-111227986-500S-1-5-21-186559946-3925841745-111227986-500Remote-ManagementShell-Unknown5668 w3wp#MSExchangePowerShellAppPool500:00:26.0389557Afficher la forêt entière : 'False', Portée par défaut : « ave.local », Configuration du contrôleur de domaine : « DC.ave.local », Catalogue global préféré : « DC.ave.local », Contrôleurs de domaine préférés : « { DC.ave.local } »False0 objects execution has been proxied to remote server.0ActivityId: a3591746-a27b-447a-b8be-ff54ae3a46f1ServicePlan:;IsAdmin:True;fr-FR"
     }
-  },
-  "Event_attributes": {
-    "xmlns": "http://schemas.microsoft.com/win/2004/08/events/event"
   }
 }