Fix integer overflow in metalayer bounds

[uwezkhan]

While looking at how metalayers are parsed in frame.c, I noticed that some of the bounds checks rely on adding values together (like offset + content_len). If those values are large enough, the addition can wrap around and the check may incorrectly pass.

This change updates those checks to use a safer pattern that avoids that situation. It also adds a small constant (FRAME_META_HDR_SIZE) instead of using a hardcoded value, so it’s a bit clearer what’s being checked.

I’ve applied the same fix in all similar places in frame.c and added a test that builds a malformed frame to make sure these cases are handled properly.

[Copilot]

Pull request overview

This PR hardens metalayer parsing in frame.c by changing bounds checks to avoid integer-overflow bypasses, introduces a named constant for the metalayer header size, and adds a regression test that crafts a malformed frame.

Changes:

• Add FRAME_META_HDR_SIZE to clarify the fixed-size overhead of a bin32 MsgPack metalayer header.
• Update metalayer bounds checks in header/trailer parsing to use an overflow-resistant comparison for content_len.
• Add a new malformed-metalayers test that attempts to trigger the overflow scenario.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
blosc/frame.c	Updates bounds checks for metalayer parsing in header/trailer.
blosc/frame.h	Adds FRAME_META_HDR_SIZE constant used by the new checks.
tests/test_frame_malformed_metalayers.c	New test intended to validate rejection of overflow-crafted metalayer lengths.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (2)

blosc/frame.c:1810

• Same as in the header-metalayer path: since FRAME_META_HDR_SIZE is now available, consider using it for the pointer arithmetic when copying the vlmetalayer payload (avoids repeating + 1 + 4 and keeps things consistent).

    if (content_len > trailer_len - offset - FRAME_META_HDR_SIZE) {
      return BLOSC2_ERROR_READ_BUFFER;
    }
    char* content = malloc((size_t)content_len);
    memcpy(content, content_marker + 1 + 4, (size_t)content_len);
    metalayer->content = (uint8_t*)content;

blosc/frame.c:1620

• Now that FRAME_META_HDR_SIZE exists, consider also using it for the content pointer arithmetic (instead of repeating + 1 + 4). This reduces duplication and ensures future format tweaks are applied consistently.

    if (content_len > header_len - offset - FRAME_META_HDR_SIZE) {
      return BLOSC2_ERROR_READ_BUFFER;
    }
    char* content = malloc((size_t)content_len);
    memcpy(content, content_marker + 1 + 4, (size_t)content_len);
    metalayer->content = (uint8_t*)content;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[FrancescAlted]

Tests are not passing. Can you have a look at this?

[uwezkhan]

@FrancescAlted can you give me any suggestions on this i am trying to improve still the files are failing

[FrancescAlted]

@FrancescAlted can you give me any suggestions on this i am trying to improve still the files are failing

I think you should explain better what you are trying to fix. My suggestion is that you start with a minimal reproducible test unit that fails; this will show clearly the issue. Then you should start trying a minimal patch for fixing the issue, until it passes all tests locally. Only after that, send the PR.

As this is open for too long and the use case is not that clear, I am going to close this. Feel free to open another PR by following the guidelines above.