Improve apple MDM parsing (#47344)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #

# Checklist for submitter

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

- [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/guides/committing-changes.md#changes-files)
for more information.

- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements), JS
inline code is prevented especially for url redirects, and untrusted
data interpolated into shell scripts/commands is validated against shell
metacharacters.
- [x] Timeouts are implemented and retries are limited to avoid infinite
loops
- [x] If paths of existing endpoints are modified without backwards
compatibility, checked the frontend/CLI for any necessary changes

## Testing

- [x] Added/updated automated tests
- [x] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)

- [x] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Enforced request body size limit for Apple MDM operations (≈16 MiB
cap).

* **Improvements**
* Safer Apple MDM plist parsing with bounds and complexity checks to
reject malformed/oversized payloads.
* Decoder updated to more strictly accept XML check-in/command payloads.

* **Tests**
* Added unit tests covering bounded plist decoding and XML-only decoding
behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Jordan Montgomery
2026-06-10 15:35:04 -04:00
committed by GitHub
parent a0b8ecea4b
commit afbbc447bb
10 changed files with 497 additions and 9 deletions
+1
View File
@@ -0,0 +1 @@
* Improved Apple MDM enrollment and device management request parsing
+4
View File
@@ -18,6 +18,10 @@ const (
// MaxMultiScriptQuerySize, sets a max size for payloads that take multiple scripts and SQL queries.
MaxMultiScriptQuerySize int64 = 5 * units.MiB
MaxMicrosoftMDMSize int64 = 2 * units.MiB
// MaxAppleMDMRequestBodySize bounds Apple MDM check-in and command-result
// request bodies. Results are stored in a MEDIUMTEXT column (max 16,777,215
// bytes), so the limit must not exceed that boundary.
MaxAppleMDMRequestBodySize int64 = (16 * units.MiB) - 1
// DefaultMaxOsqueryLogWriteSize is the default request body size limit
// applied to /api/osquery/log when osquery.allow_body_auth_fallback is
+1 -2
View File
@@ -41,7 +41,6 @@ import (
"github.com/fleetdm/fleet/v4/server/dev_mode"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/fleetdm/fleet/v4/server/mdm/apple/rootcert"
"github.com/micromdm/plist"
"github.com/smallstep/pkcs7"
)
@@ -159,7 +158,7 @@ func ParseMachineInfoFromPKCS7(buf []byte, verify bool) (*fleet.MDMAppleMachineI
}
info := new(fleet.MDMAppleMachineInfo)
if err = plist.Unmarshal(p7.Content, info); err != nil {
if err = BoundedPlistUnmarshal(p7.Content, info); err != nil {
return nil, fmt.Errorf("could not decode plist: %w", err)
}
+256
View File
@@ -0,0 +1,256 @@
package apple_mdm
import (
"bytes"
"encoding/binary"
"errors"
"fmt"
"github.com/micromdm/plist"
)
// Limits applied to binary property lists before decoding. They are generous
// for the device-info plists these endpoints handle, which are a single flat
// dictionary of scalar values.
const (
// binaryPlistMagic is the prefix that selects the binary plist decoder.
binaryPlistMagic = "bplist00"
plistTrailerSize = 32
maxPlistObjects = 1 << 16 // distinct objects (offset-table size)
maxPlistDepth = 16 // reference nesting
maxPlistNodes = 1 << 16 // objects after references are expanded
)
var (
errPlistTooComplex = errors.New("plist exceeds parsing limits")
errMalformedPlist = errors.New("malformed binary plist")
)
// BoundedPlistUnmarshal decodes a plist into v. Binary plists are first checked
// against the depth, object-count, and object-size limits above; XML plists are
// decoded directly (their size is bounded by the caller's body limit).
func BoundedPlistUnmarshal(data []byte, v any) error {
if bytes.HasPrefix(data, []byte(binaryPlistMagic)) {
if err := checkBinaryPlistBounds(data); err != nil {
return err
}
}
return plist.Unmarshal(data, v)
}
// checkBinaryPlistBounds walks a binary plist's object references, rejecting
// input that exceeds the limits or points outside the data region.
func checkBinaryPlistBounds(data []byte) error {
if len(data) < len(binaryPlistMagic)+plistTrailerSize {
return fmt.Errorf("%w: shorter than minimum size", errMalformedPlist)
}
// Trailer is the final 32 bytes (CFBinaryPlistTrailer).
trailer := data[len(data)-plistTrailerSize:]
offsetIntSize := trailer[6]
objectRefSize := trailer[7]
numObjects := binary.BigEndian.Uint64(trailer[8:16])
rootObject := binary.BigEndian.Uint64(trailer[16:24])
offsetTableOffset := binary.BigEndian.Uint64(trailer[24:32])
if offsetIntSize == 0 || offsetIntSize > 8 || objectRefSize == 0 || objectRefSize > 8 {
return fmt.Errorf("%w: invalid integer sizes", errMalformedPlist)
}
if numObjects == 0 {
return fmt.Errorf("%w: no objects", errMalformedPlist)
}
if numObjects > maxPlistObjects {
return fmt.Errorf("%w: %d objects", errPlistTooComplex, numObjects)
}
trailerStart := uint64(len(data) - plistTrailerSize) //nolint:gosec // dismiss G115, length is bounded above
tableBytes := numObjects * uint64(offsetIntSize)
if offsetTableOffset > trailerStart || tableBytes > trailerStart-offsetTableOffset {
return fmt.Errorf("%w: offset table out of bounds", errMalformedPlist)
}
offsetTable := make([]uint64, numObjects)
pos := offsetTableOffset
for i := range offsetTable {
offsetTable[i] = readUintBE(data[pos : pos+uint64(offsetIntSize)])
pos += uint64(offsetIntSize)
}
b := &plistBounder{
data: data,
offsetTable: offsetTable,
objectRefSize: objectRefSize,
dataEnd: offsetTableOffset, // objects live before the offset table
}
return b.visit(rootObject, 0)
}
type plistBounder struct {
data []byte
offsetTable []uint64
objectRefSize uint8
dataEnd uint64
nodes int
}
// visit walks the object at index, recursing into array and dict references.
// The depth and node guards keep the walk itself bounded: a cycle stops at
// maxPlistDepth, and reference expansion stops at maxPlistNodes.
func (b *plistBounder) visit(index uint64, depth int) error {
if depth > maxPlistDepth {
return fmt.Errorf("%w: nesting deeper than %d", errPlistTooComplex, maxPlistDepth)
}
b.nodes++
if b.nodes > maxPlistNodes {
return fmt.Errorf("%w: more than %d expanded objects", errPlistTooComplex, maxPlistNodes)
}
if index >= uint64(len(b.offsetTable)) {
return fmt.Errorf("%w: object ref %d out of range", errMalformedPlist, index)
}
offset := b.offsetTable[index]
if offset >= b.dataEnd {
return fmt.Errorf("%w: object offset out of range", errMalformedPlist)
}
// High nibble of the marker byte is the object type (CFBinaryPList.c).
cur := plistCursor{data: b.data, pos: offset, end: b.dataEnd}
marker, err := cur.readByte()
if err != nil {
return err
}
switch marker >> 4 {
case 0xa: // array: count object refs
count, err := cur.readCount(marker)
if err != nil {
return err
}
if count > cur.remaining()/uint64(b.objectRefSize) {
return fmt.Errorf("%w: array refs exceed input", errMalformedPlist)
}
return b.visitRefs(&cur, count, depth)
case 0xd: // dictionary: count key refs followed by count value refs
count, err := cur.readCount(marker)
if err != nil {
return err
}
if count > cur.remaining()/uint64(b.objectRefSize)/2 {
return fmt.Errorf("%w: dictionary refs exceed input", errMalformedPlist)
}
return b.visitRefs(&cur, 2*count, depth)
case 0x2: // real: 1<<(low nibble) bytes follow inline
nbytes := uint64(1) << (marker & 0xf)
if cur.pos+nbytes < cur.pos || cur.pos+nbytes > b.dataEnd {
return fmt.Errorf("%w: real size exceeds input", errMalformedPlist)
}
return nil
case 0x4, 0x5: // data, ASCII string: count single-byte units
return b.checkPayload(&cur, marker, 1)
case 0x6: // UTF-16 string: count two-byte units
return b.checkPayload(&cur, marker, 2)
default:
// Other types are fixed-size or non-recursive; nothing to bound.
return nil
}
}
func (b *plistBounder) visitRefs(cur *plistCursor, total uint64, depth int) error {
for range total {
ref, err := cur.readRef(b.objectRefSize)
if err != nil {
return err
}
if err := b.visit(ref, depth+1); err != nil {
return err
}
}
return nil
}
// checkPayload verifies a variable-length scalar (data or string) declares a
// payload that fits within the object region.
func (b *plistBounder) checkPayload(cur *plistCursor, marker byte, unitSize uint64) error {
count, err := cur.readCount(marker)
if err != nil {
return err
}
size := count * unitSize
if size/unitSize != count {
return fmt.Errorf("%w: object size overflow", errMalformedPlist)
}
if cur.pos+size < cur.pos || cur.pos+size > b.dataEnd {
return fmt.Errorf("%w: object size exceeds input", errMalformedPlist)
}
return nil
}
// plistCursor reads forward within a single object's bytes, bounded by end.
type plistCursor struct {
data []byte
pos uint64
end uint64
}
func (c *plistCursor) remaining() uint64 {
if c.pos >= c.end {
return 0
}
return c.end - c.pos
}
func (c *plistCursor) readByte() (byte, error) {
if c.pos >= c.end {
return 0, fmt.Errorf("%w: unexpected end of object data", errMalformedPlist)
}
v := c.data[c.pos]
c.pos++
return v, nil
}
func (c *plistCursor) readBytes(n uint64) ([]byte, error) {
if n == 0 {
return nil, nil
}
if c.pos+n < c.pos || c.pos+n > c.end {
return nil, fmt.Errorf("%w: unexpected end of object data", errMalformedPlist)
}
v := c.data[c.pos : c.pos+n]
c.pos += n
return v, nil
}
// readCount decodes the variable-length count used by data, strings, arrays, and dicts.
func (c *plistCursor) readCount(marker byte) (uint64, error) {
if marker&0xf != 0xf {
return uint64(marker & 0xf), nil
}
sizeMarker, err := c.readByte()
if err != nil {
return 0, err
}
nbytes := uint64(1) << (sizeMarker & 0x0f)
if nbytes > 8 {
return 0, fmt.Errorf("%w: invalid count size", errMalformedPlist)
}
buf, err := c.readBytes(nbytes)
if err != nil {
return 0, err
}
return readUintBE(buf), nil
}
func (c *plistCursor) readRef(size uint8) (uint64, error) {
buf, err := c.readBytes(uint64(size))
if err != nil {
return 0, err
}
return readUintBE(buf), nil
}
func readUintBE(b []byte) uint64 {
var n uint64
for _, c := range b {
n = n<<8 | uint64(c)
}
return n
}
+180
View File
@@ -0,0 +1,180 @@
package apple_mdm
import (
"encoding/binary"
"testing"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestBoundedPlistUnmarshalBinary(t *testing.T) {
t.Run("flat dictionary decodes", func(t *testing.T) {
data := buildFlatBinaryPlist(t, [][2]string{
{"SERIAL", "ABC123"},
{"UDID", "0000-1111"},
})
var info fleet.MDMAppleMachineInfo
require.NoError(t, BoundedPlistUnmarshal(data, &info))
assert.Equal(t, "ABC123", info.Serial)
assert.Equal(t, "0000-1111", info.UDID)
})
t.Run("nesting beyond the depth limit is rejected", func(t *testing.T) {
data := buildRefChain(t, 24)
require.Less(t, len(data), 200)
var info fleet.MDMAppleMachineInfo
err := BoundedPlistUnmarshal(data, &info)
require.ErrorIs(t, err, errPlistTooComplex)
})
t.Run("self-referential object is rejected", func(t *testing.T) {
// Object 0 is an array referencing itself (a cycle).
data := buildBinaryPlist(t, []byte{0xa2, 0x00, 0x00})
var info fleet.MDMAppleMachineInfo
err := BoundedPlistUnmarshal(data, &info)
require.ErrorIs(t, err, errPlistTooComplex)
})
t.Run("string length beyond input is rejected", func(t *testing.T) {
// ASCII string declaring a length larger than the input contains.
obj := []byte{0x5f, 0x13, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff}
data := buildBinaryPlist(t, obj)
var info fleet.MDMAppleMachineInfo
err := BoundedPlistUnmarshal(data, &info)
require.ErrorIs(t, err, errMalformedPlist)
})
t.Run("real size beyond input is rejected", func(t *testing.T) {
// Real marker 0x2f declares more inline bytes than the input contains.
data := buildBinaryPlist(t, []byte{0x2f})
var info fleet.MDMAppleMachineInfo
err := BoundedPlistUnmarshal(data, &info)
require.ErrorIs(t, err, errMalformedPlist)
})
t.Run("object count beyond the limit is rejected", func(t *testing.T) {
data := buildBinaryPlist(t, []byte{0x08}) // bool false
trailer := data[len(data)-plistTrailerSize:]
binary.BigEndian.PutUint64(trailer[8:16], maxPlistObjects+1)
var info fleet.MDMAppleMachineInfo
err := BoundedPlistUnmarshal(data, &info)
require.ErrorIs(t, err, errPlistTooComplex)
})
}
func TestBoundedPlistUnmarshalXML(t *testing.T) {
// XML plists are not reference-encoded, so bounds checking is skipped.
xml := []byte(`<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>SERIAL</key>
<string>XML-SERIAL</string>
</dict>
</plist>`)
var info fleet.MDMAppleMachineInfo
require.NoError(t, BoundedPlistUnmarshal(xml, &info))
assert.Equal(t, "XML-SERIAL", info.Serial)
}
// buildBinaryPlist wraps a single root object (placed immediately after the
// header at index 0) with a one-byte offset table and trailer.
func buildBinaryPlist(t *testing.T, rootObject []byte) []byte {
t.Helper()
body := append([]byte("bplist00"), rootObject...)
offsetTableOffset := len(body)
body = appendByte(body, len("bplist00")) // offset of object 0
return append(body, makeTrailer(1, offsetTableOffset)...)
}
// buildRefChain lays out n array objects, each referencing the next object
// twice, terminated by a single leaf object.
func buildRefChain(t *testing.T, n int) []byte {
t.Helper()
require.Less(t, n, 250, "single-byte refs require < 250 objects")
body := []byte("bplist00")
offsets := make([]int, 0, n+1)
for i := range n {
offsets = append(offsets, len(body))
body = appendByte(body, 0xa2) // array of 2 refs to the next object
body = appendByte(body, i+1)
body = appendByte(body, i+1)
}
offsets = append(offsets, len(body))
body = appendByte(body, 0x08) // leaf: bool false
offsetTableOffset := len(body)
for _, off := range offsets {
body = appendByte(body, off)
}
return append(body, makeTrailer(len(offsets), offsetTableOffset)...)
}
// buildFlatBinaryPlist builds a binary plist of a single dictionary whose keys
// and values are all short ASCII strings.
func buildFlatBinaryPlist(t *testing.T, pairs [][2]string) []byte {
t.Helper()
n := len(pairs)
require.Less(t, n, 15, "builder uses inline dict counts")
body := []byte("bplist00")
offsets := []int{len(body)}
// Object 0: dict. Key refs are objects 1..n, value refs are n+1..2n.
body = appendByte(body, 0xd0|n)
for i := range n {
body = appendByte(body, 1+i)
}
for i := range n {
body = appendByte(body, 1+n+i)
}
appendStr := func(s string) {
require.Less(t, len(s), 15, "builder uses inline string counts")
offsets = append(offsets, len(body))
body = appendByte(body, 0x50|len(s))
body = append(body, []byte(s)...)
}
for _, p := range pairs {
appendStr(p[0])
}
for _, p := range pairs {
appendStr(p[1])
}
offsetTableOffset := len(body)
for _, off := range offsets {
body = appendByte(body, off)
}
return append(body, makeTrailer(len(offsets), offsetTableOffset)...)
}
// makeTrailer builds a 32-byte trailer with single-byte offsets and refs.
func makeTrailer(numObjects, offsetTableOffset int) []byte {
trailer := make([]byte, plistTrailerSize)
trailer[6] = 1 // offset int size
trailer[7] = 1 // object ref size
putUint64(trailer[8:16], numObjects)
putUint64(trailer[16:24], 0) // root object index
putUint64(trailer[24:32], offsetTableOffset)
return trailer
}
// appendByte and putUint64 keep the fixture's narrowing conversions in one place.
func appendByte(body []byte, v int) []byte {
return append(body, byte(v)) //nolint:gosec // dismiss G115, fixture values are below 256
}
func putUint64(dst []byte, v int) {
binary.BigEndian.PutUint64(dst, uint64(v)) //nolint:gosec // dismiss G115, fixture values are bounded
}
+2 -1
View File
@@ -1,6 +1,7 @@
package mdm
import (
"bytes"
"encoding/base64"
"errors"
"fmt"
@@ -201,7 +202,7 @@ func (w *checkinUnmarshaller) UnmarshalPlist(f func(interface{}) error) error {
// DecodeCheckin unmarshals rawMessage into a specific check-in struct in message.
func DecodeCheckin(rawMessage []byte) (message interface{}, err error) {
w := &checkinUnmarshaller{raw: rawMessage}
err = plist.Unmarshal(rawMessage, w)
err = plist.NewXMLDecoder(bytes.NewReader(rawMessage)).Decode(w)
message = w.message
return
}
+3 -2
View File
@@ -1,6 +1,7 @@
package mdm
import (
"bytes"
"errors"
"github.com/micromdm/plist"
@@ -32,7 +33,7 @@ type CommandResults struct {
// DecodeCheckin unmarshals rawMessage into results
func DecodeCommandResults(rawResults []byte) (results *CommandResults, err error) {
results = new(CommandResults)
err = plist.Unmarshal(rawResults, results)
err = plist.NewXMLDecoder(bytes.NewReader(rawResults)).Decode(results)
if err != nil {
return
}
@@ -68,7 +69,7 @@ type CommandWithSubtype struct {
// DecodeCommand unmarshals rawCommand into command
func DecodeCommand(rawCommand []byte) (command *Command, err error) {
command = new(Command)
err = plist.Unmarshal(rawCommand, command)
err = plist.NewXMLDecoder(bytes.NewReader(rawCommand)).Decode(command)
if err != nil {
return
}
+44
View File
@@ -0,0 +1,44 @@
package mdm
import "testing"
// MDM check-in and command-result messages are always XML.
func TestDecodeXMLOnly(t *testing.T) {
nonXML := []byte("bplist00\xd1\x01\x02")
if _, err := DecodeCheckin(nonXML); err == nil {
t.Error("DecodeCheckin: want error for non-XML input, got nil")
}
if _, err := DecodeCommandResults(nonXML); err == nil {
t.Error("DecodeCommandResults: want error for non-XML input, got nil")
}
checkin := []byte(`<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>MessageType</key>
<string>Authenticate</string>
<key>UDID</key>
<string>00000000-1111</string>
<key>Topic</key>
<string>com.apple.mgmt.External.test</string>
</dict>
</plist>`)
if _, err := DecodeCheckin(checkin); err != nil {
t.Errorf("DecodeCheckin rejected valid XML: %v", err)
}
result := []byte(`<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>CommandUUID</key>
<string>abc-123</string>
<key>Status</key>
<string>Acknowledged</string>
</dict>
</plist>`)
if _, err := DecodeCommandResults(result); err != nil {
t.Errorf("DecodeCommandResults rejected valid XML: %v", err)
}
}
+4 -4
View File
@@ -2132,7 +2132,7 @@ type mdmAppleAccountEnrollRequest struct {
func (mdmAppleAccountEnrollRequest) DecodeRequest(ctx context.Context, r *http.Request) (interface{}, error) {
decoded := mdmAppleAccountEnrollRequest{}
rawData, err := io.ReadAll(r.Body)
rawData, err := io.ReadAll(io.LimitReader(r.Body, limit10KiB))
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "reading body from request")
}
@@ -2147,7 +2147,7 @@ func (mdmAppleAccountEnrollRequest) DecodeRequest(ctx context.Context, r *http.R
deviceInfo := fleet.MDMAppleAccountDrivenUserEnrollDeviceInfo{}
err = plist.Unmarshal(p7.Content, &deviceInfo)
err = apple_mdm.BoundedPlistUnmarshal(p7.Content, &deviceInfo)
if err != nil {
return nil, &fleet.BadRequestError{
Message: "invalid request body",
@@ -6726,7 +6726,7 @@ func (mdmAppleOTARequest) DecodeRequest(ctx context.Context, r *http.Request) (i
idpUUID := r.URL.Query().Get("idp_uuid") // Can be empty.
rawData, err := io.ReadAll(r.Body)
rawData, err := io.ReadAll(io.LimitReader(r.Body, limit10KiB))
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "reading body from request")
}
@@ -6740,7 +6740,7 @@ func (mdmAppleOTARequest) DecodeRequest(ctx context.Context, r *http.Request) (i
}
var request mdmAppleOTARequest
err = plist.Unmarshal(p7.Content, &request.DeviceInfo)
err = apple_mdm.BoundedPlistUnmarshal(p7.Content, &request.DeviceInfo)
if err != nil {
return nil, &fleet.BadRequestError{
Message: "invalid request body",
+2
View File
@@ -1459,6 +1459,8 @@ func registerMDM(
}
mdmHandler = httpmdm.CertExtractMdmSignatureMiddleware(mdmHandler, httpmdm.MdmSignatureVerifierFunc(cryptoutil.VerifyMdmSignature),
httpmdm.SigLogWithLogger(mdmLogger.With("handler", "cert-extract")))
// Bound the request body before any middleware reads it.
mdmHandler = http.MaxBytesHandler(mdmHandler, fleet.MaxAppleMDMRequestBodySize)
mux.Handle(apple_mdm.MDMPath, otel.WrapHandler(mdmHandler, apple_mdm.MDMPath, fleetConfig))
return nil
}