build-redis-from-scratch icon indicating copy to clipboard operation
build-redis-from-scratch copied to clipboard

Security Issue: Slowloris-like attack and Denial of Service/Memory exhaustion attack

Open Zelakolase opened this issue 2 years ago • 1 comments

Description

Reading the length of the array without any upper boundary could lead to large memory allocation without usage. The issue is particularly here and here. Also, I can send a legitimate request without completing it, making the thread hold and running indefinitely.

Example payload

For example, $1048576\r\nAhmed\r\n will allocate 1MB array without actually sending 1MB. Similarly, *1000\r\n$1048576\r\nhello\r\n$1048576\r\nworld\r\n.

Recommended Mitigations

  1. Put a maximum value for array allocation size as an upper boundary. This value can be calculated by determining the maximum allowed memory usage for network consumption (Can be 5% of the total available memory of the machine) and dividing it by the maximum number of threads and dividing it by two for both read/write operations.
  2. Set SO_TIMEOUT in the TCP Socket configuration. This mitigation was used for mitigating slowloris attacks. An advantage to set SO_TIMEOUT is considering a legitimate request with a large array allocation, where they sent an actual +1MB request. It's also recommended to set a backlog so that requests do not get taken down when active threads = max threads.
  3. Use dynamic memory allocation. Two advantages: a. If the client has slow connection, SO_TIMEOUT would cut the client off, b. consideration of a legitimate +1MB request. One disadvantage: A painful hit to performance (throughput rate/round-trip time/TTFB).

Notes

  1. When implementing multi-threading, please use Thread pools with Max threads = CPU cores, to enhance performance and save the time it takes to make a whole new thread, and to save time regarding CPU context switching between threads > CPU cores.
  2. I would suggest sticking with ISO25010 for code quality purposes. Anyone would have a hard time understanding the code if they didn't read the article. Code should be self-explanatory in a sense. Also, ensuring high maintainability and readability across different developers.
  3. TCP and network communication is tricky, I would suggest putting a TLS/SSL layer for security purposes (eg. MITM). Also, set SO_RCVBUF and SO_SNDBUF to be high as the maximum memory allocation for each network read operation, so fragmentation would not happen (normally). Also, set SO_KEEPALIVE to your wish!

Zelakolase avatar Apr 02 '23 13:04 Zelakolase

Hey @Zelakolase, it's been a while! I really appreciate you taking the time to share such great feedback. I’ve learned a lot from your insights, and I’ll try to find some time to apply your suggestions. I’m also open to contributions—if you or any readers would like to submit a PR to improve the series, it would be a great way to make it even more helpful for others learning from it. Thanks again!

ahmedash95 avatar Jan 30 '25 09:01 ahmedash95