Skip to content

Conversation

@AliceLR
Copy link
Contributor

@AliceLR AliceLR commented Jan 28, 2022

Patch to remove some redundant checks from #66 and #67 (favoring the ones originating from oob_read_fixes where relevant). The Oktalyzer loader was also missing chunk size bounds checks and has serious alignment/endian issues that neither patch addressed. These all should be resolved by this, but it's not necessarily the cleanest or best way.

Note: I put #pragma pack(1) on the structs since I didn't notice anything preventing ModPlug_Load from being given an unaligned pointer, which seems like a plausible thing a user could do.

  • OKT: numerous alignment/big endian fixes.
  • OKT: add missing chunk size bounds checks.
  • OKT: remove redundant chunk bounds checks added by fuzz-patch-1.
  • S3M: remove duplicate channel panning table check.
  • ULT: remove duplicate event bounds check.

* OKT: numerous alignment/big endian fixes.
* OKT: add missing chunk size bounds checks.
* OKT: remove redundant chunk bounds checks added by fuzz-patch-1.
* S3M: remove duplicate channel panning table check.
* ULT: remove duplicate event bounds check.
sezero pushed a commit to sezero/libmodplug that referenced this pull request Jan 28, 2022
* OKT: numerous alignment/big endian fixes.
* OKT: add missing chunk size bounds checks.
* OKT: remove redundant chunk bounds checks added by fuzz-patch-1.
* S3M: remove duplicate channel panning table check.
* ULT: remove duplicate event bounds check.

Konstanty#73
@Konstanty Konstanty merged commit 0431c20 into Konstanty:master Jan 28, 2022
@AliceLR AliceLR deleted the cleanup-66 branch January 28, 2022 23:40
sezero pushed a commit to sezero/libmodplug that referenced this pull request Jan 31, 2022
* OKT: numerous alignment/big endian fixes.
* OKT: add missing chunk size bounds checks.
* OKT: remove redundant chunk bounds checks added by fuzz-patch-1.
* S3M: remove duplicate channel panning table check.
* ULT: remove duplicate event bounds check.

Konstanty#73
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.

2 participants