Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bplist: teach the parser about unusual OffsetIntSize values #78

Merged
merged 2 commits into from
May 1, 2023

Conversation

MarSoft
Copy link
Contributor

@MarSoft MarSoft commented Apr 26, 2023

Fixes #77.
Notice that this is only a partial fix which works for the value of 3 and for any value < 8, but won't help in other cases. Supporting values < 16 should also be possible, but I didn't bother with it. Also this doesn't touch bplist_generator and hence won't make use of non-standard values for generating plists (while those values could probably give a benefit of slightly reduced plist size).

Fixes DHowett#77.
Notice that this is only a partial fix which works for the value of 3 and for any value < 8, but won't help in other cases.
Supporting values < 16 should also be possible, but I didn't bother with it.
Also this doesn't touch `bplist_generator` and hence won't make use of non-standard values for generating plists (while those values could probably give a benefit of slightly reduced plist size).
@DHowett
Copy link
Owner

DHowett commented Apr 26, 2023

WOW, excellent find!

This might be annoying (and I am totally okay if you tell me "no" -- I'll happily do it myself), but would you be able to write a bplist deserialization test for this case?

I quickly cross-referenced the behavior in CoreFoundation and it looks like offset int sizes that aren't powers of two are considered legal. Neat!

@DHowett
Copy link
Owner

DHowett commented Apr 26, 2023

(Also, thanks so much!)

@DHowett
Copy link
Owner

DHowett commented Apr 27, 2023

Okay, I got too excited about this and ended up writing a test

bplist_test.go

func TestBplistNonPowerOfTwoOffsetIntSizes(t *testing.T) {
	bplist := []byte{
		'b', 'p', 'l', 'i', 's', 't', '0', '0',

		// Array (2 entries)
		0xA2,
		0x01, 0x02,

		// 0xFFFFFFFFFFFFFF80 (MinInt8, sign extended)
		0x13, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x80,

		// 0x7F (MaxInt8)
		0x10, 0x7f,

		// Offset table (each entry is 3 bytes)
		0x00, 0x00, 0x08,
		0x00, 0x00, 0x0b,
		0x00, 0x00, 0x14,

		// Trailer
		0x00, 0x00, 0x00, 0x00, 0x00,
		0x00,
		0x03, // Offset Int Size 3 (not a power of 2)
		0x01,
		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03,
		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x16,
	}

	buf := bytes.NewReader(bplist)
	d := newBplistParser(buf)
	_, err := d.parseDocument()
	if err != nil {
		t.Error("Unexpected error", err)
	}
}

@DHowett DHowett changed the title Support unusual OffsetIntSize values bplist: teach the parser about unusual OffsetIntSize values May 1, 2023
@DHowett DHowett merged commit e03e84e into DHowett:main May 1, 2023
@DHowett
Copy link
Owner

DHowett commented May 1, 2023

Thanks again for the fix! I added the test case from my most recent comment and squashed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error parsing some seemingly valid binary plists (illegal integer size)
2 participants