Skip to content

Null pointer dereference in RedisCache::copyReadBuffer early-return path #82

Description

@bigsamich

Description

In RedisCache.hpp:114, the copyReadBuffer method dereferences pElementsCopied without a null check on the early-return path, even though the parameter defaults to nullptr:

RA_Time copyReadBuffer(std::span<Type> destBuffer, int firstIndexToCopy = 0, int* pElementsCopied = nullptr)
{
  // ...
  if (copySourceStart >= sourceBuffer.end()) {
    *pElementsCopied = 0;   // CRASH: pElementsCopied defaults to nullptr
    return RA_Time();
  }

Line 119 correctly checks for null, but this early-return path on line 114 does not.

Severity

High

Suggested Fix

Add a null check: if (pElementsCopied) *pElementsCopied = 0;

Activity

  1. bigsamich commented on Sep 30, 2026

    @bigsamich
    ContributorAuthor

    Still present (RedisCache.hpp is unchanged through #108/#109/#111), and the trigger is broader than a bad index.

    • Default arguments are enough. The scalar overload copyReadBuffer(Type&) passes nullptr for pElementsCopied. So reading a cache whose key has no data yet segfaults: the buffer is empty and begin()+0 == end(). ASan reports SEGV … WRITE at RedisCache.hpp:114, on main and at the stack head. With Add owned stream subscriptions and safe payload decoding #108, an empty list written to the key also leaves the cache empty, so a remote writer can trigger this.
    • A negative firstIndexToCopy isn't rejected. begin() + firstIndexToCopy is only compared against end(), so copyReadBuffer(span(dst, 4), -4, &n) reads 16 bytes from before the vector's storage (ASan heap-buffer-overflow). It then returns lastWrite as if the copy had succeeded, with *pElementsCopied reporting a full copy.

    One fix covers both:

    if (firstIndexToCopy < 0 || static_cast<size_t>(firstIndexToCopy) >= sourceBuffer.size()) {
      if (pElementsCopied) *pElementsCopied = 0;
      return RA_Time();
    }

    Then copy min(size - first, destBuffer.size()) elements using index arithmetic, never forming iterators past end(). Validated for the default-argument crash: the probe now returns ok=0 without crashing.

    Related RedisCache defects found while reviewing: #119, #120, #121.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions