From 5beeb248f7b2eb2c7cca600a3fc9a6691014b998 Mon Sep 17 00:00:00 2001
From: Ian Littman <iansltx@gmail.com>
Date: Thu, 2 Jan 2025 10:41:08 -0600
Subject: [PATCH] Handle long interned strings in MSI parsing (#25079)

For #24720. Used
https://github.com/ChaelChu/msi-props-reader/blob/master/src/msiPropsReader.ts
as inspiration. Not sure why the shift is 17 bits rather than 16 here
but confirmed that 17 works and 16 doesn't.

Tested against both existing GDrive MSIs for regression testing, plus
the one mentioned in the ticket.

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Manual QA for all new/changed functionality
---
 changes/24720-msi-large-interned-strings |  1 +
 pkg/file/msi.go                          | 20 ++++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)
 create mode 100644 changes/24720-msi-large-interned-strings

diff --git a/changes/24720-msi-large-interned-strings b/changes/24720-msi-large-interned-strings
new file mode 100644
index 000000000000..c5e559360270
--- /dev/null
+++ b/changes/24720-msi-large-interned-strings
@@ -0,0 +1 @@
+* Fixed MSI parsing for packages including long interned strings (e.g. licenses for the OpenVPN Connect installer)
diff --git a/pkg/file/msi.go b/pkg/file/msi.go
index 29538ae5d7e6..db549e9b4ea1 100644
--- a/pkg/file/msi.go
+++ b/pkg/file/msi.go
@@ -242,9 +242,25 @@ func decodeStrings(dataReader, poolReader io.Reader) ([]string, error) {
 			}
 			return nil, fmt.Errorf("failed to read pool entry: %w", err)
 		}
+		stringEntrySize := int(stringEntry.Size)
+
+		// For string pool entries too long for the size to fit in a single uint16, entry size is 8 bytes instead of 4,
+		// with the first two bytes as zeroes, the next two are the two most-significant bytes of the size, shifted
+		// 17 (?!?) bits to the left, the following two are the less-significant bits of the size, and the last two are
+		// the reference count. Verified with the OpenVPN Connect v3 installer, which has a large string blob for
+		// licenses where a 17-bit shift captures the length properly.
+		if stringEntry.Size == 0 && stringEntry.RefCount != 0 {
+			stringEntrySize = int(stringEntry.RefCount) << 17
+			err := binary.Read(poolReader, binary.LittleEndian, &stringEntry)
+			if err != nil {
+				return nil, fmt.Errorf("failed to read large string pool entry: %w", err)
+			}
+			stringEntrySize += int(stringEntry.Size)
+		}
+
 		buf.Reset()
-		buf.Grow(int(stringEntry.Size))
-		_, err = io.CopyN(&buf, dataReader, int64(stringEntry.Size))
+		buf.Grow(stringEntrySize)
+		_, err = io.CopyN(&buf, dataReader, int64(stringEntrySize))
 		if err != nil {
 			return nil, fmt.Errorf("failed to read string data: %w", err)
 		}