-
Notifications
You must be signed in to change notification settings - Fork 103
Safety guideline
Tempesta FW, as well as some other our clients' projects, is a mission critical software, working on the Internet edges. Security and reliability of the software is the key property.
The CISA Product Security Bad Practices demands a memory safety roadmap. This document addresses the requirement for our C++ code. The modern C++ provides the rich set of safe programming practices, which must be followed.
The C++ Core Guidelines provide a good set of rules to achieve C++ memory safety, in particular:
- Avoid bound errors
-
By default use
const. In Rust all variables are constant by default andmutkeyword is used to declare a variable as mutable. With this rule, we aim to achieve the same level of control over unwanted memory changes.
Typically a data plane (e.g. network packets processing) code is performance crucial, so we do use custom memory allocators, which require raw memory operations.
For such cases Rust programs must use unsafe blocks, which is equal to default C++ mode. The "default" C++ is fast, but unsafe (see Herb Sutter's keynote).
Wherever, performance isn't crucial, at least in control plane, such as configuration processing, safe, yet slower, C++ techniques must be used.
For example this unsafe C-like code:
char buf[1024];
unsigned size = sizeof(buf) - SOME_CONSTANT;
buf[size] = '\0';
read_json_config(buf, size);Should be replaces with safer:
constexpr auto size = 1024;
std::array<char, size> buf = { 0 };
read_json_config(buf, size - SOME_CONSTANT);The one problem with the original code is that it involves address arithmetics,
which is easy to make a mistake in. Another problem is that it leaves the areas
of uninitialized memory: if the JSON document is shorter than size, then there
could be uninitialized data between the end of read string and written \0.
Wherever you use * for a raw pointer, make sure that you can't use std::unique_ptr
or references &. In general, for non-performance crucial code and the code, which
doesn't need to work with raw memory, use std::unique_ptr or std::shared_ptr. E.g.
instead of
tasks[i].client = new Client(foo);use
tasks[i].client = std::make_unique<Client>(foo);Also read C++ Core Guidelines: R.3: A raw pointer is non-owning.
For example, instead of char buf[100] use std::string, std::array or std::vector.
If you still need a C-style array, use std::span or std::string_view to safely
work with it's length. Consider an example serialization function (inspired by
the blog post
and C++ Core Guidelines: Catch run-time errors early):
void
serialize(const char *str, size_t len)
{
std::cout << len << ": ";
for (auto i = 0; i < len; ++i)
std::cout << str[i] << " ";
std::cout << std::endl;
}You can call the function as
char str[] = {'a', 'b', 'c'};
serialize(str, sizeof(str));If you define str as a C-string, then you need to adjust the len computation:
char *str = "abc";
serialize(str, sizeof(str) - 1);Next, if you change the type to int, then you need other len computation:
int str[] = {'a', 'b', 'c'};
serialize(str, sizeof(str) / sizeof(str[0]));The point is that it's easy to make a bug in length computation.
C++ STL provides span and string_view to safely pass C strings and arrays
with correct length computation:
void
serialize(std::span<char> array)
{
std::cout << array.size() << ": ";
for (const auto c: array)
std::cout << c << " ";
std::cout << std::endl;
}
void
print(std::string_view str)
{
std::cout << str.size() << ": " << str << std::endl;
}
int
main()
{
char array[] = {'a', 'b', 'c'};
serialize(array);
const char *str = "abc";
print(str);
return 0;
}Or, better, use std::array and std::string (note that serialize() and
print() aren't changed and work just the same way):
std::array<char, 3> a{'a', 'b', 'c'};
std::string s("abc");
serialize(a);
print(s);Also reference C++ Core Guidelines: Prefer using STL array or vector instead of a C array for this rule.
TODO checkpatch.pl, already in CodingStyle?