Ignore non-conforming windows event version numbers (#15839) (#15860)

This causes the XML parser to ignore Version values that are not unsignedByte values (as defined in the schema).

Closes #15838

(cherry picked from commit 33f7112)

CHANGELOG.asciidoc

@@ -186,6 +186,7 @@ processing events. (CVE-2019-17596) See https://www.elastic.co/community/securit
 
 - Fill `event.provider`. {pull}13937[13937]
 - Add support for user management events to the Security module. {pull}13530[13530]
+- Made the event parser more lenient w.r.t. invalid event log definition version numbers. {issue}15838[15838]
 
 ==== Deprecated
 

winlogbeat/sys/event.go

@@ -20,6 +20,7 @@ package sys
 import (
 	"encoding/xml"
 	"fmt"
+	"strconv"
 	"time"
 )
 
@@ -36,7 +37,7 @@ type Event struct {
 	// System
 	Provider        Provider        `xml:"System>Provider"`
 	EventIdentifier EventIdentifier `xml:"System>EventID"`
-	Version         uint8           `xml:"System>Version"`
+	Version         Version         `xml:"System>Version"`
 	LevelRaw        uint8           `xml:"System>Level"`
 	TaskRaw         uint16          `xml:"System>Task"`
 	OpcodeRaw       uint8           `xml:"System>Opcode"`
@@ -203,3 +204,24 @@ func (kv *KeyValue) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
 
 	return nil
 }
+
+// Version contains the version number of the event's definition.
+type Version uint8
+
+// UnmarshalXML unmarshals the version number as an xsd:unsignedByte. Invalid
+// values are ignored an no error is returned.
+func (v *Version) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
+	var s string
+	if err := d.DecodeElement(&s, &start); err != nil {
+		return err
+	}
+
+	version, err := strconv.ParseUint(s, 10, 8)
+	if err != nil {
+		// Ignore invalid version values.
+		return nil
+	}
+
+	*v = Version(version)
+	return nil
+}

winlogbeat/sys/event_test.go

@@ -33,7 +33,7 @@ const allXML = `
   <System>
     <Provider Name="Microsoft-Windows-WinRM" Guid="{a7975c8f-ac13-49f1-87da-5a984a4ab417}" EventSourceName="Service Control Manager"/>
     <EventID>91</EventID>
-    <Version>0</Version>
+    <Version>1</Version>
     <Level>4</Level>
     <Task>9</Task>
     <Opcode>0</Opcode>
@@ -125,6 +125,7 @@ func TestXML(t *testing.T) {
 				Keywords:                []string{"Server"},
 				RenderErrorCode:         15005,
 				RenderErrorDataItemName: "shellId",
+				Version:                 1,
 			},
 		},
 		{
@@ -178,6 +179,67 @@ func TestInvalidXML(t *testing.T) {
 	assert.Equal(t, "Creating WSMan shell on server with ResourceUri: \t\r\n\\u001b", ev.Message)
 }
 
+// nonUnsignedIntVersion is an anonymized sample from a NetApp appliance that
+// produces non-conforming data.
+const nonUnsignedIntVersion = `
+<Event xmlns="http://schemas.netapp.com/events/event">
+  <System>
+    <Provider Name="NetApp-Security-Auditing" Guid="{3CB2A168-FE19-4A4E-BDAD-DCF422F13473}"/>
+    <EventID>4656</EventID>
+    <EventName>Open Object</EventName>
+    <Version>101.3</Version>
+    <Source>CIFS</Source>
+    <Level>0</Level>
+    <Opcode>0</Opcode>
+    <Keywords>0x8020000000000000</Keywords>
+    <Result>Audit Success</Result>
+    <TimeCreated SystemTime="2019-03-26T23:27:07.015494000Z"/>
+    <Correlation/>
+    <Channel>Security</Channel>
+    <Computer>anvil/vs-anvil</Computer>
+    <ComputerUUID>b1111111-2222-3444-4444-000000000000/91f49999-55fe-11e6-b525-00a098a5d936</ComputerUUID>
+    <Security/>
+  </System>
+  <EventData>
+    <Data Name="SubjectIP" IPVersion="4">192.168.1.2</Data>
+    <Data Name="SubjectHostname" Source=""/>
+    <Data Name="SubjectUnix" Uid="65534" Gid="65534" Local="false"/>
+    <Data Name="SubjectUserSid">S-1-5-21-2770437333-1905999116-9999999999-1111</Data>
+    <Data Name="SubjectUserIsLocal">false</Data>
+    <Data Name="SubjectDomainName">DOMAIN</Data>
+    <Data Name="SubjectUserName">john.doe</Data>
+    <Data Name="ObjectServer">Security</Data>
+    <Data Name="ObjectType">File</Data>
+    <Data Name="HandleID">00000000000000;00;00000000;00000000</Data>
+    <Data Name="ObjectName">(workshop_fg);/Some/Path/2020.jpg</Data>
+    <Data Name="AccessList">%%4416 %%4417 %%4418 %%4419 %%4420 %%4423 %%4424 %%1538 </Data>
+    <Data Name="AccessMask">8607</Data>
+    <Data Name="DesiredAccess">Read Data; List Directory; Write Data; Add File; Append Data; Add Subdirectory; Read Extended Attributes; Write Extended Attributes; Read Attributes; Write Attributes; Read ACL; </Data>
+    <Data Name="Attributes">Set Attributes; Create; Open a non-directory; </Data>
+  </EventData>
+</Event>
+`
+
+// TestInvalidVersion verifies that the reader will accept events where the the
+// version number is not an unsigned byte as per the schema definition.
+// Microsoft documentation defines version as:
+//
+//     <xs:element name="Version"
+//        type="unsignedByte"
+//     />
+//
+// But some event producers don't adhere to the schema. The value space of
+// xsd:unsignedByte is the range of integers between 0 and 255 — the unsigned
+// values that can fit in a word of 8 bits. Its lexical space allows an
+// optional + sign and leading zeros before the significant digits.
+//
+// Reference: https://docs.microsoft.com/en-us/windows/win32/wes/schema-version-systempropertiestype-element
+func TestInvalidVersion(t *testing.T) {
+	ev, err := UnmarshalEventXML([]byte(nonUnsignedIntVersion))
+	assert.NoError(t, err)
+	assert.EqualValues(t, 0, ev.Version)
+}
+
 func BenchmarkXMLUnmarshal(b *testing.B) {
 	for i := 0; i < b.N; i++ {
 		_, err := UnmarshalEventXML([]byte(allXML))