paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.8k stars 652 forks source link

chain_id can be abused to set a database path outside of base_path #5570

Open tmpolaczyk opened 3 weeks ago

tmpolaczyk commented 3 weeks ago

Is there an existing issue?

Experiencing problems? Have you tried our Stack Exchange first?

Description of bug

While playing around with a local testnet, I realized that if the chain_id has a /, like foo/bar, and then the database path is created in a subdirectory. So I tried to set the chain_id to ../../../../../../tmp/foo and indeed, the database was created in /tmp/foo, outside of the base_path. I don't think this is a big problem because the chain_id is usually the project name, but this should be easy to fix.

This is the code that calculates the db_path:

https://github.com/paritytech/polkadot-sdk/blob/f6eeca91b6cf0810ca3d8e7ea23988d9510851ba/substrate/client/service/src/config.rs#L280-L282

The problem is that .join() accepts any path, and there is no "push_dir" function. So we need some way to ensure that chain_id is a single directory, not a path.

Possible solutions:

Steps to reproduce

No response