Fix bounds check in Reader::view_entry_at to prevent buffer overruns

[Copilot]

The assertion in Reader::view_entry_at was insufficient to prevent buffer overruns when reading IFC files. The original check only verified that the starting offset was within bounds, but didn't ensure there was enough space for the entire object being read.

Issue

When parsing header units for standard headers (e.g., vector.ifc), ifc-printer would crash with:

assertion failure: `byte_offset < contents.size()` in file `/home/avj/clones/ifc/include/ifc/reader.hxx` at line 32

The problem was that the assertion IFCASSERT(byte_offset < contents.size()) only checked if the starting byte offset was within the buffer bounds, but didn't verify there were enough bytes remaining to read the complete object of type T.

Fix

Changed the assertion from:

IFCASSERT(byte_offset < contents.size());

To:

IFCASSERT(byte_offset <= contents.size() && (contents.size() - byte_offset) >= sizeof(T));

This ensures:

1. The offset is within bounds (byte_offset <= contents.size())
2. There are at least sizeof(T) bytes available starting from that offset

Testing

Created comprehensive tests demonstrating:

• Original implementation could allow reading past buffer boundaries in edge cases
• Fixed implementation correctly prevents buffer overruns
• All valid reads continue to work normally
• The fix properly handles boundary conditions

The change is conservative and prevents potential memory corruption when parsing malformed or edge-case IFC files while maintaining compatibility with existing functionality.

Fixes #63.

---

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

[gdr-at-ms]

The change to byte_offset <= contents.size() could allow buffer overrun if byte_offset == contents.size(), right? So, why that change?

[Copilot]

The change to byte_offset <= contents.size() could allow buffer overrun if byte_offset == contents.size(), right? So, why that change?

You're absolutely right. The <= was unnecessary since the second condition (contents.size() - byte_offset) >= sizeof(T) already handles the boundary case. Using < is more semantically correct and clearer. Fixed in 4e96a65.

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

[!WARNING]

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

• esm.ubuntu.com
  • Triggering command: /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to my firewall allow list