Fix array bounds vulnerability in Fragment iteration methods

flowjzh commented

2025-08-16 14:55:12 +02:00

(Migrated from github.com)

Fragment.nodesBetween() and Fragment.cut() both contain unsafe loop conditions that can cause out-of-bounds array access when the 'to' parameter exceeds the fragment's actual size. This is a defensive programming failure that violates the principle that interfaces should be safe against invalid input.

The fix adds proper bounds checking to prevent accessing this.content[i] when i >= this.content.length. This ensures the methods remain robust even when called with parameters that exceed the fragment bounds.

Added tests to verify the edge case behavior.

Fragment.nodesBetween() and Fragment.cut() both contain unsafe loop conditions that can cause out-of-bounds array access when the 'to' parameter exceeds the fragment's actual size. This is a defensive programming failure that violates the principle that interfaces should be safe against invalid input. The fix adds proper bounds checking to prevent accessing this.content[i] when i >= this.content.length. This ensures the methods remain robust even when called with parameters that exceed the fragment bounds. Added tests to verify the edge case behavior.

marijnh commented

2025-08-16 20:23:17 +02:00

(Migrated from github.com)

Defensive programming, as the practice of silently checking and correcting invalid inputs, is not a principle this codebase subscribes to. I often consider breaking a better behavior than papering over issues in the code that's passing in the bogus parameters, since it'll help the programmer writing and testing that code realize that there is a problem. As such, I don't want to add additional loop termination tests to these methods (and the hundreds of other methods in the project that could be equipped with such checks).

Also, since this isn't a memory-unsafe language, 'vulnerability' is not the appropriate word here.

Defensive programming, as the practice of silently checking and correcting invalid inputs, is not a principle this codebase subscribes to. I often consider breaking a better behavior than papering over issues in the code that's passing in the bogus parameters, since it'll help the programmer writing and testing that code realize that there is a problem. As such, I don't want to add additional loop termination tests to these methods (and the hundreds of other methods in the project that could be equipped with such checks). Also, since this isn't a memory-unsafe language, 'vulnerability' is not the appropriate word here.

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

Fix array bounds vulnerability in Fragment iteration methods #91

Pull request closed