Bug - Fixed visit_url POST requests send "pwd" to non-KoL servers

C

Catch-22

Active member
This has been around for ages and I'm kinda surprised bumcheekcity/fewyn/zarqon haven't noticed these arriving on their servers, as they'd collectively be running the most popular servers externally visited by ASH scripts. I don't know why it has taken me until today to report this, because it's definitely a bug and a potential security risk. I guess I've just brushed it off earlier.

The issue is in the title, but to explain it better:
> ash visit_url("http://www.google.com/?");

Server returned response code 405 for http://www.google.com/
Returned:

Now let's look at the HTTP request body:
=&pwd=mypwdstring

That's not cool :( Why does google need to know my pwd string?

I think the issue tends to become further exasperated by the request method behaviour of visit_url, GET, being switched to a POST when a querystring is present in the URL (this itself is fairly unconventional). The wiki entry isn't really accurate in that sense.

If I had done a visit_url("http://www.google.com/?pwd="); I still wouldn't want mafia to fill in my pwd string, but it wouldn't be as unexpected.

Suggested fix: Only add/autofill pwd if the host is KOL_IP/KOL_HOST. In addition, the pwd element should probably only be omitted from logs when visiting KOL_IP/KOL_HOST.

I am probably capable of patching this, but right now I am busy. If anyone else wants to take a look, feel free.
 
Last edited:
Bale

Bale

Minion
Interesting.

A password hash is assigned for a single login session and is only valid until you log out or timeout, but during that window someone else could use it to access your account.
 
Bale

Bale

Minion
fewyn and zarqon are undeniably trustworthy, but bumcheekcity's weird obsession about believing he can steal other people's souls though an unagreed EULA has always made me wary of him. Now that you posted this thread he's probably making plans to autosteal all the meat and Mr Items from everyone who runs his profile script. He can leave his script unmodified and only needs to make the changes on his server to take advantage of this security flaw. Yup, I'm definitely not running his profile script until this is patched. ;)
 
Fluxxdog

Fluxxdog

Active member
Bale said:
fewyn and zarqon are undeniably trustworthy, but bumcheekcity's weird obsession about believing he can steal other people's souls though an unagreed EULA has always made me wary of him.
Click to expand...
Heh, joke's on him. Already signed my soul over to the Army. BCC gets none ^^
 
S

slyz

Developer
Bale said:
he's probably making plans to autosteal all the meat and Mr Items from everyone who runs his profile script.
Click to expand...
My guess is that he had this plan all along. Otherwise, why would he want to compile information on what Mr. Items people have?

/goatee laugh
 
D

Darzil

Developer
slyz said:
My guess is that he had this plan all along. Otherwise, why would he want to compile information on what Mr. Items people have?
Click to expand...

It's worse than that, he's going to steal even the bound items, by proving that he owns our souls, so they are clearly bound to him !
 
C

Catch-22

Active member
Well, this is long overdue but I finally got around to it.

Catch-22 said:
Suggested fix: Only add/autofill pwd if the host is KOL_IP/KOL_HOST. In addition, the pwd element should probably only be omitted from logs when visiting KOL_IP/KOL_HOST.
Click to expand...

Basically I have done the above, but instead of checking if the host is "KOL_IP/KOL_HOST" I went with what the existing code does to determine if it's an external request. As an added bonus, it also fixes a(n?) NPE.
 

Attachments

  • GenericRequest.java.patch
    2.2 KB · Views: 43
Last edited:
B

bordemstirs

Member
Bale said:
Interesting.

A password hash is assigned for a single login session and is only valid until you log out or timeout, but during that window someone else could use it to access your account.
Click to expand...

Is this actually a possibility? I've noticed a lot of newer/redone kol pages don't make use of pwd and I assumed it was because they started using something more secure (or at the very least more convenient: like PHPSESSIONID).
We should test this!
 
C

Catch-22

Active member
bordemstirs said:
Is this actually a possibility?
Click to expand...
Yes, it's possible. As a proof of concept, shortly after I disclosed this bug I hijacked my own multi session and sent myself fraudulent KMail. Doing that from a remote computer would be a little more involved, but still possible.

Edit: Also I thought I had posted this earlier in the thread, but I hadn't. KoL doesn't discriminate against your IP address, you can change it mid-session and you will not be asked to login again, provided you have the same pwd hash. This may have changed a little with the new load balancing stuff, I haven't tinkered with it in a while, but I believe it still works roughly the same way.

bordemstirs said:
I've noticed a lot of newer/redone kol pages don't make use of pwd and I assumed it was because they started using something more secure (or at the very least more convenient: like PHPSESSIONID).
Click to expand...

I've noticed that KoLmafia uses the pwd on KoL in a few places where it's not actually needed, whether or not it was needed at some point in the past I couldn't say. I know there are certain places that absolutely still need the pwd to be present.

Possibly something else to note is that I believe the PHPSESSIONID gets logged in the debug log. We're currently redacting the pwd string from logs, is it worth doing the same for PHPSESSIONID? By the time someone posts a log there's probably not a whole lot you could do with it, but the same is true for pwd strings.
 
Last edited:
Z

zeroToNine

Member
The way I understand is KOL always uses a session ID cookie to find out what account your GET / POST is associated with.

The pwd parameter is for additional security, so you, for example, cannot post a link in chat that, when clicked by careless/clueless people, will kmail all their Mr. Accessories to your account. Not all actions are deemed "worthy" of this additional measure, so not all KoL PHP scripts will check for it.
 
You must log in or register to reply here.
Top