He asked: why is there no argument to memcpy() to specify the maximum destination length?
That’s the third one.
If you really insist, #define safe_memcpy(d, s, dn, sn) memcpy(d, s, min(dn, sn))?
Yeah, also, I don’t understand why would they want that.
Imagine calling memcpy(d, 10, s, 15), and having your data not copied entirely, having your d buffer with cropped data. Garbage, essentially. How would that be better?
edit: to be clear, I’m not complaining about your suggestion, but about the reasoning of the presenter on this.
Yeah, also, I don’t understand why would they want that.
Imagine calling memcpy(d, 10, s, 15), and having your data not copied entirely, having your d buffer with cropped data. Garbage, essentially. How would that be better?
Cropped data would be a logic error in your application. With standard memcpy the additional 5 bytes overwrite whatever is in memory after the d buffer. This can even enable an attacker to introduce execution of their own code. That’s why ie. Microsoft ships a memcpy_s.
Reading materials:
But the unanswered question is why you’re calling memcpy(d, s, 15) instead of memcpy(d, s, 10)? At some level the problem is calling the function with the wrong argument, and adding more arguments maybe doesn’t help.
Every security exploit can be drilled down to “why were you doing this!”. If there was an obvious answer, security exploit would have been a thing of the past. Meanwhile advocating harm reduction is as good as we can get because even if calling memcpy with a smaller destination is wrong to begin with, truncated data still has a more chance to end up with non-exploitable crash than plain old buffer overflow that often end up with reliable code exec.
But why do we assume this extra parameter is better than the other parameter which we have assumed is incorrect? Why not add another extra parameter? memcpy_reallysafe(dest, src, destsize, srcsize, destsize_forserious, doublechecksize)
Because in ten years a line of code can change and the assumptions that made one variable the right one will break. Suddenly you got the wrong variable in there. Personally, I think this is where asserts belong, to codify the assumptions over a long span of time and multiple developers.
A common use case of memcpy is to copy a buffer over another. The way program are structure we often end up with srcsize and dstsize that matches their buffer. The error come from the implicit contract that srcsize is always at least bigger than dstsize. Sure, good code would ensure this is always true. Actual code had many instance where it is not. Adding dstsize to memcpy means that this contract is now explicit and can be asserted by the actual function that put this contract in place.
I mean, at this point we are not arguing of hypothetical scenario, we have a whole history of this bug class happening over and over again. Simply keeping track of the semantic (Copy one buffer to the other) and asking for all the properties required (Buffer and their size) is a low effort and easy way to prevent many of those bug.
Yeah, keeping track of the buffer size is a very good idea. But if you want it to always be correct, it should be done without requiring the programmer to manually carry the buffer size along in a separate variable from the buffer pointer.
Either something like “Managed C++”, where the allocator data structures are queried to figure out the size of the buffer, or something like Rust slices:
typedef struct {
char *ptr;
size_t len;
} slice_t;
slice_t slice(slice_t slice, size_t start, size_t end) {
assert(start <= end);
assert((end - start) <= slice.len);
slicet.ptr += start;
slice.len = end - start;
return slice;
}
slice_t slice_front(slice_t slice, size_t start) {
assert(start <= slice.len);
slice.ptr += start;
slice.len -= start;
return slice;
}
slice_t slice_back(slice_t slice, size_t end) {
assert(end <= slice.len);
slice.len = end;
return slice;
}
void slicecpy(slice_t dest, slice_t src) {
assert(dest.len == src.len);
memcpy(dest, dest.len, src);
}
The point being to make it harder to mix up which len goes with which ptr, plus providing a assert-assisted pointer manipulation in addition to the safe memcpy itself. A safe abstraction needs to account for the entire life cycle of its bounds check, not just the point of use.
Also, this would really, really benefit from templates.
Signal is a messaging app, with a central provider. I would thing something more like Freenet would be ideal for moving PGP keys around. Freenet is slow, but PGP keys are tiny and infrequently downloaded, Freenet tries to maintain availability for a file as long as someone occasionally requests it (even if the original uploader goes away), and a Freenet node operator can’t tell what’s on their machine unless they already know what they’re looking for.
At least, if the problem you’re trying to overcome is the fact that SKS server operators can be compelled to censor them. Personally, I would think a project like Fedora would prefer to make their SKS server invite-only and only have package maintainers put their keys up there. If Fedora put up a Freenet node, they’d probably be censored by every country that engages in that kind of thing, and I’m not sure if they want to fight that fight.
I feel like there are basically two types of dependencies: for the sake of bikeshedding, call them A and B.
Type A dependencies are things like PGP, your kernel, your web browser, your SSL implementation, your Acme/Let’s Encrypt client, your MTA, ‘sanitize-html’, etc: things that talk to the outside world and/or pose a security risk.
Type B dependencies are things like ‘react-loading-spinner’, your MUA, your terminal emulator, etc. These are things that don’t talk to the outside world, don’t touch secrets, and don’t have significant security implications.
I want my package manager to be able to either realise for itself or be told that type A dependencies should be upgraded as soon as possible. I want it to pester and harass me about updates to these sorts of dependencies. I also think you should be meticulously careful about introducing these sorts of dependencies. The latter kind of dependencies are different: throw them in willy-nilly, do what you like with them, leave them for years or don’t, up to you. Never auto-update them.
If you maintain a type A dependency you have a responsibility to ensure that you do maintain backwards compatibility. People should be able to rely on you. They (and in this case NPM) should be able to auto-update type A dependencies without thinking about it really.
Because really, npm updating sanitize-html is not a bad thing. Do you want to sit around on an old insecure version of a library that sanitizes HTML? Presumably you want your HTML sanitised for a reason.
There is no such thing as a library that categorically “does not touch secrets”. A terminal emulator has to render a mix of secret data and untrusted data, react-loading-spinner could be replacing itself with a widget full of sensitive data and have access to that for a brief moment, and I’m honestly surprised that you think an email client is type B (the mail server that it talks to is trusted, but the email payload is where most of the complexity is, and it’s not trusted data).