ruby / ruby/rbs

[Security Issue] rbs_string_strip_whitespace reads past empty ranges

Open Beginner friendly
#3,108 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ruby
Stars
2.2k
Forks
256
Avg merge
6d 17h
Merged PRs (30d)
37

Description

Summary

rbs_string_strip_whitespace dereferences the start of a string range before checking whether the range is empty. An empty range therefore produces an out-of-bounds read under AddressSanitizer. The parser calls this helper for annotation bodies, whose grammar permits an empty body (<>).

Affected current master: 5de6ecda19eaec2bd225db301674dc650dc220c3
Affected release checked: RBS 4.2.0 (8aee3b6fa6231dbd6fefe01f2ba20e3104228a6f)

Source

The current loop condition reads *new_start before evaluating new_start < self->end:

while (isspace(*new_start) && new_start < self->end) {
    new_start++;
}

The trailing-whitespace loop uses the same unsafe evaluation order. Call sites are in integer-literal and annotation parsing.

Reproduction

This small harness invokes the helper with a valid empty half-open range at the end of a one-byte allocation:

#include <stdlib.h>
#include "rbs/string.h"

int main(void) {
    char *storage = malloc(1);
    rbs_string_t empty = rbs_string_new(storage + 1, storage + 1);
    rbs_string_t stripped = rbs_string_strip_whitespace(&empty);
    free(storage);
    return stripped.start == stripped.end ? 0 : 1;
}

Compile with AddressSanitizer against src/string.c and run it. On macOS/arm64 with Apple clang 17.0.0:

ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1
    #0 rbs_string_strip_whitespace
    #1 main
0 bytes after 1-byte region

Suggested fix

Check the range before dereferencing it, and cast through unsigned char before passing bytes to isspace:

-while (isspace(*new_start) && new_start < self->end) {
+while (new_start < self->end && isspace((unsigned char) *new_start)) {
...
-while (isspace(*new_end) && new_start < new_end) {
+while (new_start < new_end && isspace((unsigned char) *new_end)) {

With that change, the ASan harness exits successfully. The native extension compiles on Ruby 4.0.6, and test/rbs/type_parsing_test.rb passes 40 tests / 525 assertions. I have not established code-execution impact; the demonstrated failure is an out-of-bounds read on an empty parser string range.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/string.c at rbs_string_strip_whitespace and review its callers in integer-literal and annotation parsing. Run the supplied AddressSanitizer harness, then test/rbs/type_parsing_test.rb; the work is done when empty ranges no longer cause an out-of-bounds read and the listed tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, ruby
Domain
security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
85/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.