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

0 stars 0 forks source link

http requests can be accessed at the local address 127.0.0.1 #53

Closed c4-bot-8 closed 3 months ago

c4-bot-8 commented 3 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#L109

Vulnerability details

Impact

Some services will open the local port, allowing only 127.0.0.1 access, for example, provide some administrative pages, which can be set up through the api service.

Since only local access is allowed, these services may not have permission controls, or may use default passwords.

In addition to local services that communicate directly over the LAN via server clusters, some services may provide http services to allow other machines on the LAN to access them.

Pink Extension does not restrict the destination address when making http requests, The user can access any port on the worker machine, as well as other addresses within the worker's LAN. The attacker can resolve the domain name to 127.0.0.1 or another address on the LAN.

The attacker can also use the http request function to scan the worker and obtain the internal information of the worker, such as the internal port and network structure.

Proof of Concept

async fn async_http_request(
    request: HttpRequest,
    timeout_ms: u64,
) -> Result<HttpResponse, HttpRequestError> {
    if timeout_ms == 0 {
        return Err(HttpRequestError::Timeout);
    }
    let timeout = Duration::from_millis(timeout_ms);
    let url: reqwest::Url = request.url.parse().or(Err(HttpRequestError::InvalidUrl))?;
    //@audit A Url can point to any address
    let client = reqwest::Client::builder()
        .trust_dns(true)
        .timeout(timeout)
        .env_proxy(url.host_str().unwrap_or_default())
        .build()
        .or(Err(HttpRequestError::FailedToCreateClient))?;

    let method: Method =
        FromStr::from_str(request.method.as_str()).or(Err(HttpRequestError::InvalidMethod))?;
    let mut headers = HeaderMap::new();

    for (key, value) in &request.headers {
        let key =
            HeaderName::from_str(key.as_str()).or(Err(HttpRequestError::InvalidHeaderName))?;
        let value = HeaderValue::from_str(value).or(Err(HttpRequestError::InvalidHeaderValue))?;
        headers.insert(key, value);
    }

    let result = client
        .request(method, url)
        .headers(headers)
        .body(request.body)
        .send()
        .await;
    .....
}

Tools Used

vscode, manual

Recommended Mitigation Steps

Add a blocklist to block access to certain addresses. First, perform domain name resolution to obtain the ip address, and then filter ip addresses.

Assessed type

Other

c4-pre-sort commented 3 months ago

141345 marked the issue as primary issue

c4-pre-sort commented 3 months ago

141345 marked the issue as sufficient quality report

141345 commented 3 months ago

localhost access and abuse

kvinwang commented 3 months ago

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

c4-sponsor commented 3 months ago

kvinwang (sponsor) disputed

OpenCoreCH commented 3 months ago

The warden has pointed out a valid SSRF concern that could lead to information leakage (or even worse outcomes if services are configured to treat requests from some subnets differently, for instance by not requiring authentication). The solution that the sponsor uses in practice is an external proxy server, which is appropriate and used often for similar issues. However, because the codebase was audited in isolation and this proxy was neither part of the codebase nor documented, the issue applies to the code in scope. I am therefore judging it as a valid medium.

c4-judge commented 3 months ago

OpenCoreCH marked the issue as selected for report

DadeKuma commented 3 months ago

I strongly believe this issue should not be considered a valid Medium for several reasons.

Since only local access is allowed, these services may not have permission controls, or may use default passwords

1) It's essential to recognize the scope boundaries set for this contest. Third-party services fall outside this scope, and suggesting they might be unprotected is speculative without concrete evidence. Proving their security is impossible within the contest's constraints. Unless the Warden can prove that local HTTP requests can be used to impact the Pink runtime itself in any way, this is probably OOS.

2) Adding these types of checks to the business backend logic goes against the best security practices, which is why they are not implemented here. No amount of checks can be considered secure in this context. This responsibility typically falls under the purview of a Web Application Firewall, which is also out of scope for this contest.

The attacker can also use the http request function to scan the worker and obtain the internal information of the worker, such as the internal port and network structure

3) This issue shows no proof of how the previous statement can be applied due to the lack of evidence. Again, this is pure speculation that some local web service could be polled with HTTP requests to extract some valuable data if it isn't protected by a password. AFAIK, it's not possible to simply 'extract data' with a local HTTP request unless there is an active web service that exposes that data through a specific port. However, please feel free to prove me wrong if this is not the case.

For these reasons, I think this issue should be considered invalid.

kvinwang commented 3 months ago

2. Adding these types of checks to the business backend logic goes against the best security practices, which is why they are not implemented here. No amount of checks can be considered secure in this context. This responsibility typically falls under the purview of a Web Application Firewall, which is also out of scope for this contest.

Agreed. Even if we opt to include a built-in ACL module for outgoing network requests, the optimal decision would be to implement it on the worker side rather than within the pink-runtime.

OpenCoreCH commented 3 months ago
  1. It's essential to recognize the scope boundaries set for this contest. Third-party services fall outside this scope, and suggesting they might be unprotected is speculative without concrete evidence. Proving their security is impossible within the contest's constraints. Unless the Warden can prove that local HTTP requests can be used to impact the Pink runtime itself in any way, this is probably OOS.

  2. This issue shows no proof of how the previous statement can be applied due to the lack of evidence. Again, this is pure speculation that some local web service could be polled with HTTP requests to extract some valuable data if it isn't protected by a password. AFAIK, it's not possible to simply 'extract data' with a local HTTP request unless there is an active web service that exposes that data through a specific port. However, please feel free to prove me wrong if this is not the case.

You are right about that. From my PenTesting background, I always consider it a potential problem when you can access internal network resources over a publicly exposed service because it is extremely common to have servers behind a firewall (typically with NAT) that only exposes the internal ports which are really used. And in many server setups, you actually have services that are only accessible internally or treat requests from localhost (or the same subnet / all private subnets) differently. That's why SSRF is a pretty common issue (even in the OWASP Top 10 2021).

However, it was wrong from my side to act under this assumption. In the end, this was an audit of the runtime and the runtime itself cannot be attacked by this, only in particular setups. But if we would start making these assumptions, the same would apply to findings about potential deployments with things like wrong library versions, outdated OS versions, etc..., which is also OOS for such an audit. So I agree with you, great creativity on the warden's side, but unfortunately OOS.

c4-judge commented 3 months ago

OpenCoreCH marked the issue as unsatisfactory: Out of scope