code-423n4 / 2024-03-phala-network-findings

0 stars 0 forks source link

Lack of Rate Limiting for HTTP Requests #13

Open c4-bot-7 opened 6 months ago

c4-bot-7 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/chain-extension/src/lib.rs#L56-L98

Vulnerability details

Impact

The impact of this vulnerability could be severe. An attacker could exploit the absence of rate limiting to launch DoS attacks, causing service degradation or complete unavailability. By flooding the system with a large volume of requests, the attacker could exhaust server resources such as CPU, memory, or network bandwidth, rendering the system inaccessible to legitimate users.

Proof of Concept

pub fn batch_http_request(requests: Vec<HttpRequest>, timeout_ms: u64) -> ext::BatchHttpResult {
    const MAX_CONCURRENT_REQUESTS: usize = 5;
    if requests.len() > MAX_CONCURRENT_REQUESTS {
        return Err(ext::HttpRequestError::TooManyRequests);
    }
    // Send HTTP requests concurrently...
}

pub fn http_request(
    request: HttpRequest,
    timeout_ms: u64,
) -> Result<HttpResponse, HttpRequestError> {
    // Send a single HTTP request...
}

In the batch_http_request function, there is a check to limit the number of concurrent HTTP requests (MAX_CONCURRENT_REQUESTS). However, there is no mechanism for rate limiting the overall volume of requests over time. This means that while the number of requests sent concurrently is limited, there is no control over the rate at which requests can be sent overall. Without rate limiting, an attacker could still flood the system with a large number of requests sequentially, bypassing the concurrent request limit and potentially causing a denial of service (DoS) attack.

Similarly, in the http_request function, there is no rate limiting mechanism implemented. Each call to this function represents a single HTTP request, and there is no restriction on the frequency or volume of these requests. Therefore, an attacker could repeatedly call this function to send a large number of requests within a short period, overwhelming the server and causing service disruption.

Tools Used

Manual

Recommended Mitigation Steps

Rate limiting mechanisms should be implemented at the application level to control the rate of incoming HTTP requests. This can involve setting limits on the number of requests per second, per minute, or per hour, depending on the desired level of protection and the system's capacity.

Assessed type

Error

c4-pre-sort commented 6 months ago

141345 marked the issue as primary issue

c4-pre-sort commented 6 months ago

141345 marked the issue as sufficient quality report

141345 commented 6 months ago

number of concurrent HTTP requests is resitricted, but not for frequency of HTTP requests

kvinwang commented 6 months ago

This work should be implemented on the worker side, not at runtime. Actually, there are already some things, such as CFS-like Scheduler scheduling network IO, in the worker to handle such impacts.

kvinwang commented 6 months ago

We had a discussion when designing this and decided to have a professional external firewall program handle the system level network security, which is why all outgoing requests are supported to set a SOCKS proxy server.

c4-sponsor commented 6 months ago

kvinwang (sponsor) acknowledged

OpenCoreCH commented 6 months ago

The fact that the runtime does not implement rate limiting does not necessarily lead to a DoS opportunity, this heavily depends on how the runtime is invoked from the workers and what the measures to limit resource usage are there. Implementing rate limiting as an additional safe guard in the runtime is a valid suggestion, therefore downgrading to QA

c4-judge commented 6 months ago

OpenCoreCH changed the severity to QA (Quality Assurance)

c4-judge commented 6 months ago

OpenCoreCH marked the issue as grade-b