KomodoPlatform / dPoW

Komodo delayed Proof of Work software
36 stars 73 forks source link

VRSC update v1.2.2-2 #554

Open Asherda opened 3 months ago

Asherda commented 3 months ago

Commit: https://github.com/VerusCoin/VerusCoin/commit/dee30f36c31aa60dfd6cfbd5b5ebf21cfafa2169 Release: https://github.com/VerusCoin/VerusCoin/releases/tag/v1.2.2-2

DeckerSU commented 2 months ago

LGTM, except for the newly introduced encryption feature (RPC signdata with the encrypttoaddress parameter), which could potentially be exploited by attackers.

I.e., I'm trying to point out that when the signdata functionality was limited to hashing messages (or even arbitrary files in the file system), it was somewhat acceptable. However, with the addition of data encryption features, verusd can now be considered a third-party encryption tool. From certain perspectives, this might be considered unsafe.

Exactly, anyone with access to the RPC can read and encrypt any file in the file system that verusd can access... and then do whatever they want with the encrypted data. Of course, if someone has already gained access to the system, they could access everything, but it's still kind of alarming. I see additional possibilities for malicious use of verusd in this way.

Let's imagine a scammer forces a user to show the output of:

./verus signdata '{"createmmr":true, "filename":"'$HOME'/.komodo/VRSC/wallet.dat", "encrypttoaddress":"scammer_z_address"}'

p.s. Despite the fact that @miketout mentioned he has already committed a parameter to enable file encryption, making it default to true for hashes and false for file contents, which will be active starting from the next release, I feel it's important to mention my concerns here anyway.

DeckerSU commented 2 months ago

p.p.s. Also, from my perspective, server-side RPC implementation should not interact with the filesystem at all due to security concerns. In other words, there shouldn't be any RPCs through which the daemon produces any files on the server or, conversely, reads the contents of any files on the server. If someone needs an "encryption tool," for instance, to encrypt a gif picture and tie it to an identity, there should be a separate tool for that. This functionality should not be a part of the daemon's capabilities. Please consider this as merely my own humble opinion.

ca333 commented 2 months ago

LGTM, except for the newly introduced encryption feature (RPC signdata with the encrypttoaddress parameter), which could potentially be exploited by attackers.

I.e., I'm trying to point out that when the signdata functionality was limited to hashing messages (or even arbitrary files in the file system), it was somewhat acceptable. However, with the addition of data encryption features, verusd can now be considered a third-party encryption tool. From certain perspectives, this might be considered unsafe.

Exactly, anyone with access to the RPC can read and encrypt any file in the file system that verusd can access... and then do whatever they want with the encrypted data. Of course, if someone has already gained access to the system, they could access everything, but it's still kind of alarming. I see additional possibilities for malicious use of verusd in this way.

Let's imagine a scammer forces a user to show the output of:

./verus signdata '{"createmmr":true, "filename":"'$HOME'/.komodo/VRSC/wallet.dat", "encrypttoaddress":"scammer_z_address"}'

p.s. Despite the fact that @miketout mentioned he has already committed a parameter to enable file encryption, making it default to true for hashes and false for file contents, which will be active starting from the next release, I feel it's important to mention my concerns here anyway.

agree - since its running on 3P should be "fine" however. Approving this as well.

p.s. VRSC mentioned not needing dPoW security wise and has protection covered with pbaas/notarization. i.e. with the upcoming S8 we ll disable dPoW for this asset which ll significantly reduce maintenance requirement as a side effect. @Asherda plz let us know if this is fine.

gcharang commented 2 months ago

announced: https://github.com/KomodoPlatform/dPoW/pull/557