Don't return `userpass` on first request without auth

[lukechilds]

We discussed this in Slack but just wanted to track it here to make sure it doesn't get missed out.

Current behaviour is that if you start the marketmaker daemon from the CLI with your passphrase then the first request it gets it will respond with the userpass. The user pass is used for authentication for all subsequent requests.

This means when a user spawns the marketmaker process, during the time it is running but before they have interacted with it, any website they visit has the ability to create a localhost request, grab their userpass and then proceed to use their userpass to make authenticated requests to withdraw all of the funds they hold for all of their currencies to an address of the website's choosing.

This can be avoided by not blindly trusting the first request is non-malicious and providing the userpass. Instead authentication should always be required via the passphrase API to obtain the userpass.

[jl777]

this is already fixed with hardcoding of the initial command line passphrase to being "default" it will always return the identical userpass and does not pose any security issue. it does make the initial api return useless and if it can be simply removed without breaking any existing GUI, that is a good idea. however I am not sure what codebase is out there that relies on this

[lukechilds]

I would definitely vote for removing the initial API response all together. Your current change is already a breaking change (it'll break our GUI).

this is already fixed with hardcoding of the initial command line passphrase to being "default" it will always return the identical userpass and does not pose any security issue

If I'm understanding correctly it's still technically open to the web, just the user's funds/seed phrase are now safe. Any third party website could use the original hardcoded userpass to pass in a custom pass phrase of their own and then start running loads of full nodes on a users machine which they would be able to control etc.

I would suggest the best solution would be to allow starting the daemon with a seed phrase, and only allow anyone to connect if they can prove they also know that seed phrase. e.g How you can currently with the existing passphrase api.

[jl777]

your assumption that remote access can be done is not correct only very few commands are allowed from non-localhost ip

I think when everybody transitions to the current hardcoded "default" passphrase and using passphrase api, then I can safely deprecate the initial response

[lukechilds]

When I say "open to the web" I mean any website can run malicious JavaScript that can make localhost requests to marketmaker, which it will accept.

[jl777]

yes, ok. I will deprecate it in the upcoming etomic fork, actually I already did so there wont be anymore initial userpass and all the code that burns a command to skip past it will still work

[lukechilds]

I just upgraded to the latest build of mm and it looks like all that's changed is the userpass is no longer given out to whoever makes the initial connection but it's still using the same hardcoded value which will successfully auth for the first request.

This doesn't fix anything. If someone's writing malicious JavaScript to target mm they'll just also include the hardcoded userpass.

Referring back to two points I made previously in this thread:

This can be avoided by not blindly trusting the first request is non-malicious

I would suggest the best solution would be to allow starting the daemon with a seed phrase, and only allow anyone to connect if they can prove they also know that seed phrase.

This is what's needed to fix this vulnerability.

[jl777]

I dont understand what the vulnerability is? some malicious site can spend the funds in the "default" passphrase's address?

or some malicious site will put its own passphrase in and take froms from its address?

as it is now, if the malicious site submits a passphrase api first, then it could lock out the user, but no user funds are at risk. OR the user submits the passphrase first and the malicious site cannot access.

[lukechilds]

The vulnerability is that a malicious website can access a local daemon. Surely you can understand why that would be a risk?

There could be multiple attack vectors in any of your endpoints. mm should make sure that only the intended user has access to do anything with it, not just give access to the first person that pings it.

For example a malicious website can run a Komodo full node on the victims machine. That's going to take up users resources and is obviously not a good thing.

I just tested the swapstatus command and it looks like it's just reading the most recent swaps saved to disk without checking if they were issued by the userpass that's currently authenticated. So this means a malicious website can read the users trade history, which contains TXIDs, from those they can get the users address and balance. That's a pretty huge privacy issue.

These are the first two commands that come to mind, and are pretty bad on their own, I'm sure more issues exist.

And those two examples are using the commands in the way that they are supposed to be used. mm is written in a memory unsafe language so buffer overflows are inevitably going to happen. If someone puts in the time to study your source this could be much worse. A simple coding error on your end could result in remote access to the users filesystem, memory or even arbitrary code execution.

All you need to do is implement proper authentication and the attack surface goes down massively. It would probably even be simpler code on your end. The current authentication is over complicated and doesn't really achieve much.

Currently you've got:

• Init daemon
• Client needs to know the hardcoded default daemon userpass
• Client needs to know the passphrase
• Client sends hardcoded default daemon userpass along with passphrase
• Daemon inits addresses from passphrase and returns new userpass
• Client saves new userpass and can make future requests with this

Once someone starts the daemon (step 1) any malicious website can take over the rest of the process.

And what's the point of the userpass token if we need to send over the passphrase to get a token anyway? Both client and server need knowledge of the passphrase and we need to send it between them to get the userpass. So the userpass token is kind of pointless.

Why not follow the advice I mentioned before:

This can be avoided by not blindly trusting the first request is non-malicious

I would suggest the best solution would be to allow starting the daemon with a seed phrase, and only allow anyone to connect if they can prove they also know that seed phrase.

The user should init the daemon with their passphrase. The "userpass" token should just be a hash of the passphrase.

This means the auth process goes like this:

• Init daemon with passphrase
• Client needs to know the hash of the passphrase
• Client can make requests with the hash of the passphrase

Way simpler and way more secure. Don't process any commands unless they can provide the token.

This stops any untrusted execution because you can be sure that if they can provide the token, which is a hash of the passphrase, then someone who has knowledge of the plain text passphrase generated it for them, and therefore whoever issues the request is trusted.

This also means that the client doesn't need any knowledge of the passphrase, you could build a web client and users could generate their token from their passphrase and paste it in, the website doesn't need plain text knowledge of the passphrase.

This also means the passphrase never needs to be transmitted, which I'm sure you'll agree is a good thing. It's the most sensitive data we have and is the key to all of the users funds across all of their currencies in BarterDEX and Agama. It should just be used to init mm and generate the keys, once that's done we really shouldn't be passing it around between processes. That's just asking for it to end up sitting in a plaintext log file somewhere on the users machine.

[jl777]

ok, I removed the change that hardcodes the command line passphrase to "default"

now you need the passphrase or userpass to issue any further api calls

I will make it accept either the userpass it returns or a simple sha256 of the passphrase as userpass. this is not there yet but I will get that done soon

[jl777]

latest version allows the SHA256(passphrase) in hexstr form to be used as userpass

[lukechilds]

Thanks, just tested the latest build and it looks good.