On January 17th 2023, X41 and Gitlab published a report of the source code audit they performed on Git (funded by the OSTIF foundation).
This post is based on the (great) report available here and aims to investigate how Rust mitigates some of the vulnerabilities shown in this report, but also to put some light on what it doesn't mitigate by itself, and how a programmer can address these issues using good practices.
The role of these kinds of studies are primordial, and the OSTIF allows to fund such initiatives.
Please for the sake of great opensource software and computer security consider making a donation.
If I say anything that is wrong / oversimplified, do tell me please and I will correct this article in consequence. The explanations are intentionally kept simple, please refer to the corresponding report section for more information. This vulnerability (described at section 4.1.1) is an Uncontrolled ResourceConsumption (CWE 400), leading to a possible Denial of Service. When the input text grows, the value of Let's try to reproduce the same kind of loop in Rust now: This code compiles without any warning and gives you a nice infinite loop indeed. Lesson number one: Rust doesn't protect from casting overflows if you cast naively using This vulnerability (described at section 4.1.2) is an Out of Bound Read (CWE 125), allowing possible sensitive data reading, or even a buffer overflow. When dealing with strings, the computer maps the data in memory as: Notice the However imagine This vulnerability exists because the code forgets to dereference the returned pointer (pointing to somewhere in the string) when performing a condition. Because this condition will be always true, it allows the execution of a function with invalid inputs, leading to the vulnerable behavior. In Rust, you have to use an explicit You should not use unsafe Rust as long as you don't have a very specific reason to use it. This vulnerability (described at section 4.1.3) is an Integer overflowto Buffer overflow (CWE 680), that could lead to code execution. The issue here is that in Windows 64bit, the size of an Indeed, we can overflow the bounds of the In the case when Now let's do something similar in Rust shall we ? Several things on this: This kinds of issues can definitely happen in Rust if you use The issue may exist in Rust, but the memory vulnerability doesn't (at least in safe Rust), attempts to write something and run some code would lead to the program termination. This vulnerability (described at section 4.1.4) is a Synchronous Accessof Remote Resource without Timeout (CWE 1088), leading to a possible Denial of Service. This issue is really simple to understand. When initiating a new connection for a If an attacker open X connections to endpoints he controls (and that doesn't send any data), then the connections are kept active in Rust is not really protected by that kinds of things, and it's to the attention of the developer to pay attention to always put some kind of boundaries to the connections, whether it is a hard limit of simultaneously opened connections, a timeout, etc ... Timeouts can be annoying to set up, but they may really be worth it as there is nothing as unexpected as a bad Internet connection, a crash from a distant server, or any kind of I/O scenario a sane programmer wouldn't think of. Even a long one is useful for these cases. This vulnerability (described at section 4.1.5) is an Inefficient RegularExpression Complexity (CWE 1333), leading to a possible Denial of Service by consumming excessive CPU resources. This vulnerability happens because somewhere in the code, the user input gets interpreted as a regular expression. See the OWASP article about this kind of attacks. For the regex crate, the reference when dealing with regular expressions in Rust, the security is taken seriously and some features at the root cause of this kind of problems are simply not implemented. That mean that the crate is less powerful than other systems out there, but it is a tradeoff that the developers chose to make. See the "Untrusted inputs" section from the docs of the crate for more details This vulnerability (described at section 4.1.6) is a Heap-based BufferOverflow (CWE 122), leading in the worst case scenario to arbitrary code execution. This is also a vulnerability caused by the overflow of the type However here this is much more problematic as it allows to set a negative offset, and so perform a Heap overflows may not be as bad as Stack overflows, but they do have really nasty exploit possible (see CTF101 article about it). Now would that kind of things be possible in a world covered in (safe) Rust ? However you would have to choose between different types to use, meaning that for this situation to happen, you would have to explicitly write I decided to count this as a protection, as the amount of work required to make this behavior happen assures that you brought it on yourself. This vulnerability (described at section 4.1.7) is another Heap-basedBuffer Overflow (CWE 122), similarly leading in the worst case scenario to arbitrary code execution. This is yet another Here the recommendation for this issue is to use a However as this is can be only exploited for memory manipulation, this is where (safe) Rust protects us. It becomes critical because of the possible arbitrary code execution following, however this could be used in Rust to create infinite loops, trigger conditions, etc ... This vulnerability (described at section 4.1.8) is an UncontrolledResource Consumption (CWE 400), leading to a possible Denial of Service. The report isn't really clear about this, but when applying a patch, this code gets triggered: The vulnerability here comes from an issue making the It's caused by yet another integer overflow, when parsing a binary patch file. With a header / payload long enough, you can overflow the variables, and the return value from the function gets overflowed: As an This is a case that could totally apply to a Rust code if we are not careful enough, the best protection is to use proper numeric types to ensure a size doesn't get negative, but you should also learn to notice the conditions that would make any loop go infinite, and check for these conditions. Remember that the Rust "safety" is only relative and under particular conditions, it's not the same if you are writing for embedded systems, or in the Linux kernel (as Linus explained in some of the mails), and it only works if youset up everything Rust needs to achieve safety Rust is implemented so it eliminates undefined behaviors, and handles the "wrong answer" case by returning an error, or panicking. This is a choice that makes total sense when building a software / an application, but it's not a universal "best way to do", it depends on what you do. Let's review some good practices (generally speaking) in Rust to ensure we don't hit too much errors, or create some vulnerabilities. For a quick and dirty cast, However a best practice is to never use it and rely on the As This isn't specific to Rust, but why using Input sanitization is important, and the size of the input is one aspects of it, you should never overlook it as it can lead to nasty behaviors. You may think that "it will never happen, to have a blog post title larger than 65536 characters", but an attacker will think of this case, and break your code. If you use some unsafe in your Rust code, be very careful of what is written inside, make it as little as possible, as tested as possible, and only if you cannot make it using another way. It may be ok to use unsafe blocks in these situations: If something breaks in an unexpected way, the unsafe parts of the code must become primary suspects, so keep it as clear, simple and documented as possible. If the use of In any case, if you do have In both cases (and many other kind of examples), you need to think of "What happens if the whole Earth wants to connect to my app ?" Answer is, your server will crash, or melt. So put in place some limits to the number of users, or an (inexpensive) waiting queue from which users will be redirected once a "resource unit" will be available. I personally think that you can have applications serving millions of users running on a tiny server, if you smartly designed the way your resources are being used, and put in place ways to improve the behavior of your software under stress. Rust by itself, when not using any unsafe, is preventing against some vulnerabilities, including 2 Critical, 1 high and 1 medium. However it didn't protect against 4 Low vulnerabilities. Note that most of the protection was not because the vulnerability didn't occur, but more because it's not exploitable, or at least with less critical impact. This is what the rules of Rust concerning memory manipulation protects you from. However as the issues causing these vulnerabilities can still happen, you can still have vulnerabilities, and may have critical ones if you only rely on "Rust is safe". Rust in most cases is memory safe, but not all exploits are about memory exploitation, nothing is always safe, always doubt the security of a software, whether you code it or buy it. The bottom line is that you should be careful when writing code, especially when handling system's inputs (whether it is human or network / disk). You may think that it's only meant for big opensource project and that your code is OK without all of this, but I think it's important to train as these "good practices" only become automatic if repeated enough. So try to think a little about security when building the next (blazing fast) GNU tool rewritten in Rust, or anything else really. You can make a donation to the OSTIF fund by following this link. The report is accessible here and the summary can be seen on X41's website . Once again, if you find anything that is wrong / oversimplified in this article, please tell me so I can correct it right away. (computing)Detailing some vulnerabilities
I'll be focusing on the basics (that should prevent me from saying too many wrong statements), and how these issues could be seen in a program made in Rust.GIT-CR-22-01
The issue is caused by this loop occurring in one of the functions:while
slen
does as well, and researchers succeeded to allocate 2.5GB of memory using a 30MB file.let mut slen: u64 = + 1;
while slen > 0
However note the 2 casts that we are required to perform in order to get to this point, it's much easier to notice that something may break here, but on a large codebase with an example much more complex, it may still be occurring.as
. However you can use the try_into
function for casting that will return a Result<T, T::Error>
triggered if such things happen.GIT-CR-22-02
Letters: "r" "u" "s" "t"
Bytes: 0x72 0x75 0x73 0x74 0x00
0x00
byte at the end ? That's how the computer can detect the end of the string.
As the numeric value for this byte is 0
, in C we detect it using:int next_char = ;
if
get_next_char
doesn't return the value, but a pointer to this value, then !char
doesn't check if the value is 0
anymore, but it checks if the pointer is a NULL
pointer, which it won't be even if its value may be 0
.unsafe
block to perform operations on raw pointers like this, and that doesn't give you the "full power".
As I am too unfamiliar with unsafe Rust, I won't try to reproduce this here.GIT-CR-22-03
unsigned long
is 4 bytes, whereas it is 8 bytes on Linux 64bit.
As git
has been created with Linux in mind, in the following code, something goes wrong:size_t msg_A_len = ;
size_t msg_B_len = ;
unsigned long new_buffer_len = msg_A_len + msg_B_len + 2;
unsigned long new_buffer_len
, looping back to 0
. Now we can get big inputs that make the new_buffer_len
variable small.
Now imagine the remaining of the code would be something like:char* new_buffer = ;
;
;
new_buffer_len < (msg_A_len + msg_B_len)
, that would mean we just wrote more bytes into memory than the allocated memory was ableto contain, we just overflowed the buffer.println!;
// size of usize: 8
let msg_a_len: u64 = u64MAX >> 1;
let msg_b_len: u64 = u64MAX >> 1;
assert_eq!;
let new_buffer_len: usize = + + 3;
// debug compilation: thread panick here because of integer overflow
let some_array: = Vec with_capacity;
println!;
// release compilation: 1
x86_64
machine, meaning usize
is the same length as an u64
,so this code does not reproduce what actually happens in this vulnerability.debug
mode (and can be set/unset in thecompilation profile)Vec
here, which has all the safeguards tonot overflow. Arrays are not possible here as their size has to be knownat compile time.usize
) is obvious to the developer anddoesn't change across platforms, this is by itself a great protection as longas we pay attention to the types we use. (let size: i32
is not a good practiceat all.)as
castings all the time, and even if the memory size of variable is much more simple in Rust than in C, usize
size in memory is arch-dependant (as described in the usize type documentation).GIT-CR-22-04
git clone
operation, no timeout is set, meaning that if the remote endpoint doesn't answer, the connection is kept open.git
, the resources aren't freed, leading to a Denial of Service.GIT-CR-22-05
Using this, an attacker can pass a nasty Regexp that cause a denial of Service, called ReDos in that case.GIT-CR-22-06
int
when getting numbers bigger than its bounds and loops back to the minimum bound, in this case negative:size_t len = ;
size_t padding = ;
int offset = padding - len;
;
memcpy
operation before the start of the buffer, and write data controlled by the attacker.
We saw that Rust doesn't always protect against numeric types overflow.i64 offset
, as usize
is unsigned and neither are u8 u16 u32 u64
.
Using unsafe
, it is possible to get similar problems leading to possible vulnerabilities as well, for example in this code:let input_string = String from;
let strlen: usize = input_string.len;
let bufflen: usize = 10;
let buffer = String with_capacity;
let offset: i64 = - ;
let ptr = buffer.as_mut_ptr;
unsafe
Thanks to
u/pluuth
for pointing out the correct unsafe code here, I previously thought it was much more complex to get an issue like this to appear in Rust. I still count it as "protected" because of the mandatory unsafe
block and the type casts you need to use.GIT-CR-22-07
int
type overflow when handling big inputs (displaying how important this is), however this vulnerability is really critical as now an attacker can commit a malicious .gitattributes
file into a remote repository, and the vulnerability will be triggered to anybody trying to clone
or pull
the repository.size_t
type in order to prevent the integer overflow, however it's also pointed out to limitthe size of the lines in the .gitattributes
file.GIT-CR-22-08
// apply.c:4687
static int
parse_chunk
returns 0, resulting in an infinite loop.// apply.c:2124
static int
int
can be overflowed to a negative value, a malicious patch can return0
here, and performing a git apply
over the patch would result in an infinite loop, and a Denial of Service.Good security practices in Rust code
And the reality is that there are no absolute guarantees. Ever.
The "Rust is safe" is not some kind of absolute guarantee of code safety. Never has been.Not completing the operation at all, is not really any better than getting the wrong answer, it's only more debuggable.
... ]
So this is something that I really need the Rust people to understand. That whole reality of "safe" not being some absolute thing, and the reality that the kernel side requires slightly different rules than user space traditionally does.Casting overflow
as
is fine, as long as you are sure of your bounds.From
and TryFrom
traits.
If you upcast u32
to u64
, you can use .into()
as From<u32>
is implemented for u64
, and when you downcast u64
to u32
, use .try_into()
instead, it will return an error if you overflow the bounds of the integer.
The performance cost for the usage of these is negligible / null, so you should always use them for a clean and secure code.usize
size in memory is arch-dependant (see the docs), I advice to use numeric variable types that have a fixed memory space, like u64
, i32
or f32
, as much as possible to reduce the possibility of an integer overflow panick.
That is even more true if you expect your code to run on different architectures.Limit input size
u64
and checking for overflows everywhere when you can simply limit any input length is < u8::MAX
? (Or whatever type you use)Unsafe Rust
unsafe
really improves the performances, add some benchmarks in order to prove it, and if one day the gap between the safe
and unsafe
implementation is getting close, consider moving back to the all-safe implementation.unsafe
in your code, test it extensively, plug your CI to perform the tests before any merge, and make sure all of it is well tested.Limit the scale of your software
Vec
every time someone connects to your server,this is a resource consumption.
A lot of resources are available online to learn how to deal with these issues, so I won't give you any naïve advices here, do your researches.Closing thoughts
Rust's protection
In code or in life, apply Defense in depth, know the strengths and weaknesses of the technology you use, and keep your mind open :-)Writing secure Rust code
This is where extensive tests are useful (parametric tests for example), and in general the more the attacker can control an input in the vulnerable code, the more he can exploit it badly.
Get used to write secure code by default.Donate to OSTIF
Special thanks to
u/Rodrigodd_
for pointing out some things to improve in the articleu/Shnatsel
for pointing an imprecision in GIT-CR-22-03
's conclusionu/milliams
for correcting some typosu/ssokolow
for correcting some typos and grammar mistakes, and details about unsafe benchmarking and testing@teor2345
for giving a precision on casting, allowing to improve the good practices recommendations on casting.@myers
, @Arriv9l
for a correcting a typo@pepsiman
for correcting several typos over the whole article