llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.93k stars 11.94k forks source link

[Stack protector] Unnecessary stack cookie checking #56718

Open JianxiaoLuIntel opened 2 years ago

JianxiaoLuIntel commented 2 years ago

The flag -fstack-protector inserts a guard variable (aka stack cookie) onto the stack frame for each vulnerable function. For example, if a function will write a buffer, after the buffer is written, the stack cookie need to be checked to make sure the buffer overflow didn't happen.

Consider the following test case:

#include <cstring>
#include <cstdlib>
#include <cstdio>
#pragma warning(disable : 4996)   // for strcpy use

// Vulnerable function
__attribute__((noinline)) int vulnerable(const char* str , int a) {
    if (a % 2 == 0)
    {
                // Write buffer
        char buffer[10];
        strcpy(buffer, str);
        return buffer[0];
    }
    return 0;
}

int main() {
    // declare buffer that is bigger than expected
    const char* str = "This string is longer than 10 characters!!";
    printf("%d\n", vulnerable(str, std::rand()));
}

The function is vulnerable because it may write the buffer, so there will be a stack cookie on the stack frame. If it enter the branch which write the buffer, the stack cookie need to be checked to avoid buffer overflow. If it enter another branch which won't write any buffer, ideally, it's unnecessary to check the cookie. However, we found it will be checked in fact.

Here is the assembly code: On Windows: clangwin

On Ubuntu: clangubuntu

By the way, I wonder whether we can inline the __security_check_cookie on Windows.

llvmbot commented 1 year ago

@llvm/issue-subscribers-backend-x86

The flag **-fstack-protector** inserts a guard variable (aka stack cookie) onto the stack frame for each vulnerable function. For example, if a function will write a buffer, after the buffer is written, the stack cookie need to be checked to make sure the buffer overflow didn't happen. Consider the following test case: ```C++ #include <cstring> #include <cstdlib> #include <cstdio> #pragma warning(disable : 4996) // for strcpy use // Vulnerable function __attribute__((noinline)) int vulnerable(const char* str , int a) { if (a % 2 == 0) { // Write buffer char buffer[10]; strcpy(buffer, str); return buffer[0]; } return 0; } int main() { // declare buffer that is bigger than expected const char* str = "This string is longer than 10 characters!!"; printf("%d\n", vulnerable(str, std::rand())); } ``` The function is vulnerable because it may write the buffer, so there will be a stack cookie on the stack frame. If it enter the branch which write the buffer, the stack cookie need to be checked to avoid buffer overflow. **If it enter another branch which won't write any buffer, ideally, it's unnecessary to check the cookie. However, we found it will be checked in fact.** Here is the assembly code: On Windows: ![clangwin](https://user-images.githubusercontent.com/84117673/180915102-6dca620d-3713-446d-a751-24db145c07bf.png) On Ubuntu: ![clangubuntu](https://user-images.githubusercontent.com/84117673/180915119-66337b93-6357-4a57-a9bf-647c98a045c5.png) **By the way, I wonder whether we can inline the __security_check_cookie on Windows.**