Remove memcpy-UB in fbstring
authorNicholas Ormrod <njormrod@fb.com>
Mon, 10 Oct 2016 22:45:25 +0000 (15:45 -0700)
committerFacebook Github Bot <facebook-github-bot-bot@fb.com>
Mon, 10 Oct 2016 22:53:37 +0000 (15:53 -0700)
commit7c2ef80cad4449b84a0eb2ee147c818b76256f73
tree6305bc272c58f37789976332eeb2c063c914cc67
parent328c1a56226af7219787981940d9f9b05faf4aea
Remove memcpy-UB in fbstring

Summary:
The bug report in t13764686 shows that `folly::StringPiece().str()` causes undefined behavior by passing nullptr into memcpy: the default-initialized StringPiece has two null pointers, which will call `string(char*, size)` with nullptr and 0. This gets forwarded to `podCopy` in fbstring, which calls memcpy and has undefined behavior.

The call pattern `string(nullptr, 0)` is not illegal (note: that syntax is ambiguous, as the literal `0` can be converted to a pointer). Prevent this legal call pattern from causing undefined behavior in fbstring.

This diff puts asserts in podCopy to catch errors, and ensures that current podCopy callsites ensure that the pointer is non-null.

Most podCopy callsites do not need to guarded, since the size is guaranteed to be greater than 0 (which will hard crash instead). The interesting callsite is in `initMedium`, because there is a mode (disableSSO) which will allocate empty strings on the heap, so we need to add a guard there.

Reviewed By: luciang

Differential Revision: D3996440

fbshipit-source-id: b311a311973d1d969542245c72035c5b38da58e3
folly/FBString.h
folly/test/FBStringTest.cpp