Admin API can cause arbitrary file write [BugBounty][Security issue]

[Shashank-In]

Describe the bug As per the documentation, the following API is used to dump the current memory footprint of the node to the specified file.

curl -X POST --data '{
    "jsonrpc":"2.0",
    "id"     :1,
    "method" :"admin.memoryProfile",
    "params" :{
        "fileName":"mem.profile"
    }
}' -H 'content-type:application/json;' 127.0.0.1:9650/ext/admin

I noticed in the code that the file name is not sanitized, which is a user-supplied input.

Hence a malicious attacker abuse this to overwrite an existing file on the server like main.go file Also, an attacker can overwrite files of other directories by suppling the fileName parameter as

"fileName":"../../some_config_file"

Impact A malicious user can overwrite an existing file which is required for the functioning of the application in the same or different directory. This could crash the application or maybe lead to command execution. For now, at least a configuration file can be overwritten with a memory dump value and make the file useless.

To Reproduce Tested on localhost mac. Find the steps and observations below.

1. File name as file.txt

POST /ext/admin HTTP/1.1
Host: 127.0.0.1:9650
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:76.0) Gecko/20100101 Firefox/76.0
Accept: application/json, text/plain, */*
Accept-Language: en-GB,en;q=0.5
Accept-Encoding: gzip, deflate
Content-Type: application/json;charset=UTF-8
Content-Length: 143
Origin: https://wallet.ava.network
Connection: close
Referer: https://wallet.ava.network/create

{
    "jsonrpc":"2.0",
    "id"     :1,
    "method" :"admin.memoryProfile",
    "params" :{
        "fileName":"file.txt"
    }
}

File created at $GOPATH/src/github.com/ava-labs/gecko/

File name as ../../file.txt

POST /ext/admin HTTP/1.1
Host: 127.0.0.1:9650
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:76.0) Gecko/20100101 Firefox/76.0
Accept: application/json, text/plain, */*
Accept-Language: en-GB,en;q=0.5
Accept-Encoding: gzip, deflate
Content-Type: application/json;charset=UTF-8
Content-Length: 143
Origin: https://wallet.ava.network
Connection: close
Referer: https://wallet.ava.network/create

{
    "jsonrpc":"2.0",
    "id"     :1,
    "method" :"admin.memoryProfile",
    "params" :{
        "fileName":"../../file.txt"
    }
}

File created at $GOPATH/src/github.com/

So we can even overwrite files of other directories as well.

Expected behavior The file name should be sanitized and accept only alphabets (and maybe one ".")

Screenshots

Operating System MacOS Catalina

By submitting this issue I agree to the Terms and Conditions of the Developer Accelerator Program.

[moreati]

Short term suggested mitigations

1. Run ava as a dedicated user
2. Run with ava -http-host localhost to restrict network access to the RPC interface (won't help if running wallet on the same host)
3. Run ava in a container such as Docker, or as a systemd service with namespaces applied (note to self, add that to #151)

These are by no means fixes, and I'm not contesting the severity of this issue. I don't speak for AVA Labs.

[Shashank-In]

@moreati Nope that won't be the best fix. The fix here would be to sanitize the input value of the file name.

[moreati]

Nope that won't be the best fix.

I totally agree, that's why I wrote "These are by no means fixes".

[moreati]

Brainstorming possibility fixes

1. Don't accept a filename, generate one and return the path to the caller
   • Too inflexible?
2. Don't allow path separators in the filename argument, always write a known directory (e.g. $TMP, ~/.gecko/profiles, ava -profiles-dir /some/path)
   • Determining path separator is platform dependant. It introduces complexity
   • How to determine the directory?
   • Still subject to symlink redirection attacks?
3. Restrict filename to a known safe character set (e.g. [a-zA-Z0-9_-]+)
   • Internationalisation/localisation issues?

[danlaine]

Fixed by #256. Namely:

• Admin API is closed by default
• HTTP port is only exposed via NAT traversal if HTTP server is specified as being something other than localhost
• Profiles are written to a fixed file name

Thanks again for your work on this :) @Shashank-In @moreati