ast icon indicating copy to clipboard operation
ast copied to clipboard

nv_putval() is calculating the wrong size of the buffer/string to move

Open krader1961 opened this issue 7 years ago • 0 comments

When I started working on this project a year ago one of the first things I did was try to build and run it on macOS which is largely based on BSD. One of the first hard failures I saw was the memcpy() in nv_putval() failing because the buffers overlapped. Not knowing at the time if the buffers could legitimately overlap I made the simple fix and replaced memcpy() with memmove().

Fast forward to today when we now have ASAN to help us find problems. It reported that the memmove() sometimes accesses memory beyond the end of the buffer pointed to by sp. That happens because dot at that junction is from nv_size(np) which returns 12 and the sp buffer is typically 9 bytes long.

Note that even though the sp buffer is 9 bytes long it contains a single character; i.e., a string of length one. There is a magic strlen() + 8 involved in the allocation of the buffer. What the magic 8 means is anyone's guess. It wouldn't surprise me if it was added solely to deal with the memcpy() size argument being too large.

I am now 98% certain that all the places where we replaced memcpy() with memmove() do not intentionally involve overlapping buffers. Rather, the size argument is too large. If the two buffers are from malloc() and happen to be adjacent then it can appear they overlap due to the too large size argument.

krader1961 avatar Jul 12 '18 01:07 krader1961