CppCoreGuidelines
CppCoreGuidelines copied to clipboard
Discourage implicit casting from/to void pointer
In both ES.48: Avoid casts and Pro.safety: Type-safety profile I see no mention regarding the danger of (implicitly or explicitly) casting from/to void pointer. Since it strips type info away, the caller can do abysmal things to the pointer type. Example:
void *suppress_warning(void *ptr)
{
return ptr;
}
int main()
{
gsl::owner<char *> a = new char{};
volatile auto *p = static_cast<double *>(suppress_warning(a));
*p = 3; // <--- Detected by ASan: heap-buffer-overflow (WRITE of size 8)
delete a;
}
This would be equivalent of a single reinterpret_cast
/ C-style cast, except it's not covered by C++ Core Guidelines.
Currently, clang-tidy doesn't generate a warning for it. If this feels obvious enough to catch by just looking at the code block, it becomes more subtle when the void pointer is used to hold a resource that is returned from a C-style API.
Example of returning resource with void pointer:
// The API documents that it returns an owning resource of
// either char or double based on the provided argument(s).
gsl::owner<void *> get(bool flag)
{
if (flag) {
return new char{}; // <--- This is the returned value
} else {
return new double{};
}
}
int main()
{
gsl::owner<volatile double *> p = static_cast<double *>(get(true));
*p = 3; // <--- Detected by ASan: heap-buffer-overflow (WRITE of size 8)
delete p;
}
Source: https://github.com/trailofbits/clang-cfi-showcase/blob/23405d0e5b5aa4f1e7c456628ec3e0a09be32694/cfi_unrelated_cast.cpp#L34
On a side note, this is also the only C++ example in that project which clang-tidy failed to provide any meaningful warning and had to resort to runtime checks (ASan / CFI).
The issue seems to be only fixable from the interface developer side, there isn't much that can be done from the user side apart from outright disallowing casts from void pointer (which would sadly make those unsafe API unusable).
For C++ codes, instead of void pointer, the interface developer can use std::variant
for returning different types of pointer:
auto get(bool flag) -> std::variant<gsl::owner<char *>, gsl::owner<double *>>
{
if (flag) {
return new char{};
} else {
return new double{};
}
}
int main()
{
gsl::owner<volatile double *> p = std::get<gsl::owner<double *>>(get(true));
*p = 3;
delete p;
}
This results in a std::bad_variant_access
exception and therefore blocks the user from misusing the API.