Can we remove usage of memcpy?

[craxal]

clipboard-files uses memcpy, which is not an API we should be using.

The most likely fix to this is to use std::string instead of std::shared_ptr<unsigned char>.

https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1864180

[JasonYeMSFT]

We need to directly manipulate memory to parse the file paths out of consecutive memory chunks. Using string doesn't help much because at the end of the day, we still need to tell a string constructor to take bytes from a given address with a size and, like memcpy, the std::string constructor doesn't check the boundary of the source buffer. For example, the following code can be compiled and will result in undefined behavior:

char x[] = "hello"; // 6 bytes including the null terminator
std::string s(x, 7); // Construct the string with 1 byte overflow
std::cout << s << std::endl;

The memcpy usage in our code are only for platforms where the secure alternatives aren't available. They also have size validation that ensures neither the source nor the destination buffer would overflow.

[craxal]

@JasonYeMSFT If there are no alternatives, we need to make sure that we've used the proper annotations and reviewed with security team. We're not done until then.

[JasonYeMSFT]

Another reason we don't want to use a string is that we need to do in-place decoding on the raw data, which would better be done on a raw byte buffer. Although C++ strings are mutable, since the mutation may change the defacto size of the string, it would be hard for us to maintain a valid state of the string object. For example, we may end up getting a string whose .length() method returns a value greater than the actual length of the string. As far as I know, the security reviewers are fine with this approach if there is nothing else we can do.

[JasonYeMSFT]

Anyway, I managed to use string class to handle the memory management for us. Our code no longer has any usage of memcpy now.