JustArchiNET / ArchiSteamFarm

C# application with primary purpose of farming Steam cards from multiple accounts simultaneously.
Apache License 2.0
11.11k stars 1.05k forks source link

Add IPC Host to "Access-Control-Allow-Origin"-Header #701

Closed MrBurrBurr closed 6 years ago

MrBurrBurr commented 6 years ago

Bug

Description

When you change IPC Host to the IP (like 192.168.0.5) of the server where ASF is running and send an IPC command you are not able to receive the IPC answer because of line 110 in IPC.cs

Line 110 from IPC.cs: context.Response.AppendHeader("Access-Control-Allow-Origin", "null");

Here we need to change "null" into the IPC Host if its not 127.0.0.1.

Example: context.Response.AppendHeader("Access-Control-Allow-Origin", "Config.IPC.Host");

or

context.Response.AppendHeader("Access-Control-Allow-Origin", "http://192.168.0.5");

Expected behavior

Browser should be able to receive IPC answer.

Actual behavior

Browser is blocked from IPC answer because "Access-Control-Allow-Origin" Header is equal to the IP of the server where ASF is running.

Steps to reproduce

Run ASF from server A. Change IPC Host to IP of server A. Send IPC command from client B. ASF will run the IPC command but the answer wont be delivered to client B.

JustArchi commented 6 years ago

cc @KlappPc - I'm not sure if this won't break your stuff, since you added it the last time. Let me know if changing null to IPCHost suits you.

KlappPc commented 6 years ago

Wait. (wrote a different comment first ^^)

Just figured out, that what @MrBurrBurr wrote is wrong.

You would need to add the IP of Server B.

Requests from the IPC hosts are always allowed, since thats not cross-origin. At least thats how it's supposed to work. Currently do not have a webserver to test it.

So changing it to IPCHost, would not make the above case work, it would just break the local case.

So probably make that option configurable.

There has to stand the IP of the Server SENDING the request, if and only if it differs from the IPCHost.

Again, at least thats how it's supposed to work.

JustArchi commented 6 years ago

That's also my understanding.

I feel like just specifying * and calling it a day, since somebody else might come up with a server A and clients B and C. Anything wrong in this?

KlappPc commented 6 years ago

The only problem with "*" is that ANY website you visit can send commands using javascript (if ASF runs locally).

So even if my ASF is not reachable from the internet someone could send commands if I visit his website.

But I am not sure if that is problem. Probably sending 100s of !lootall commands to trigger a ban might be the worst case.

But then you still have to run it in server mode, which might not be important for like 99% of users. (Probably changes if a good javascript UI is done.)

If it's configurable, people still can enter "*", but I guess the config is cluttering up much, these days ^^

JustArchi commented 6 years ago

That's a wrong solution then, but so is everyone else 🤔.

I'm wondering if we can't under any circumstance make it automatic. I thought about specifying client's IPv4/IPv6 in that header but then it wouldn't be any different from your security standpoint either. If nobody comes up with any clever solution then I guess I'll just make it configurable.

KlappPc commented 6 years ago

Specifiying "*" and adding an "IPCPassword" would probably fix all problems.

So a config option to add a password, that has to be send with each command for it to be valid.

Then it doesn't matter if some is able to send commands (either via a Website your visiting or by getting the IP of your public reachable ASF).

To execute commands they at least have to sniff out transfered data.

But I'm not sure if thats not shooting brids with canonns ;->

JustArchi commented 6 years ago

I like this solution since it can also benefit normal usage as an extra step, but I'm still not sure if I want to make CORS * by default when IPCPassword will be empty, oh well, gonna add a note to the wiki I guess.

JustArchi commented 6 years ago

You can check if V3.0.5.0 addresses this properly, as well as if IPCPassword works, since I didn't even bother testing my code before release, I'm that lazy 🙂

5aaee9 commented 6 years ago

@JustArchi I think it should return HTTP 401 unauthorized if password is wrong 😮

JustArchi commented 6 years ago

Agreed, I'll change it before stable.