Fix buffer overflow in ForceAscii when processing unaligned pointers

[simonla]

Description

There is a bug in ForceAscii() at line 208 of src/nbytes.cpp that causes a heap/stack buffer overflow when the source pointer is unaligned.

Root Cause

When src is unaligned (src_unalign > 0) and both src and dst have the same alignment offset, the code calculates:

const unsigned unalign = bytes_per_word - src_unalign;
ForceAsciiSlow(src, dst, unalign);
src += unalign;
dst += unalign;
len -= src_unalign;  // BUG: should be "len -= unalign"

The variable unalign represents the number of bytes processed to align the pointers, but len is incorrectly decremented by src_unalign instead of unalign.

On a 64-bit system (bytes_per_word = 8), if src_unalign = 2:

• unalign = 8 - 2 = 6 bytes are processed
• But len is only decremented by 2 instead of 6
• This leaves len 4 bytes larger than the actual remaining data
• Subsequent reads in the word-aligned loop or remainder handling will overflow

Fix

Change line 208 from:

len -= src_unalign;

to:

len -= unalign;

How to Reproduce

Build with AddressSanitizer and run the test:

mkdir build_asan && cd build_asan
cmake .. -DCMAKE_CXX_FLAGS="-fsanitize=address -g" \
         -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address"
make -j4
./tests/basic --gtest_filter=basic.force_ascii_unaligned

Test case:

TEST(basic, force_ascii_unaligned) {
  const size_t buf_size = 24;
  char* src_buf = new char[buf_size];
  char* dst_buf = new char[buf_size];
  
  for (size_t i = 0; i < buf_size; ++i) {
    src_buf[i] = static_cast<char>(0x80 | i);
  }
  
  // Offset by 2 bytes to create unaligned pointers
  char *src = src_buf + 2;
  char *dst = dst_buf + 2;
  
  nbytes::ForceAscii(src, dst, 22);  // Triggers overflow
  
  delete[] src_buf;
  delete[] dst_buf;
}

ASAN output:

ERROR: AddressSanitizer: heap-buffer-overflow on address ...
READ of size 1 at ... in nbytes::ForceAsciiSlow
... is located 0 bytes after 24-byte region

[simonla]

Should we bump to NodeJS?

I don’t know much about NodeJS—its memory might be aligned, and perhaps it has V8 sandbox protection

@lemire

[lemire]

I have issued a release of nbytes. It should get picked up by node.

You have fixed a bug, but the bug requires specific alignment, so it is not clear whether it can impact Node.js itself. My suspicion is that it does not. Still, we should push the fix.