Skip to content

Commit

Permalink
Handle long interned strings in MSI parsing (#25079)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
iansltx committed Jan 2, 2025
1 parent 4c463b6 commit bd93cc3
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
1 change: 1 addition & 0 deletions changes/24720-msi-large-interned-strings
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Fixed MSI parsing for packages including long interned strings (e.g. licenses for the OpenVPN Connect installer)
20 changes: 18 additions & 2 deletions pkg/file/msi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit bd93cc3

Please sign in to comment.