Security flaw

I have discovered a possible data execution security flaw in kolmafia scripts. Be wary of scripts obtained from others which use visit_url(). I am going to send Holatuwol a full bug report in a non public form. I do not know if it is a true issue or not, but until Holatuwol says different be wary.
 

holatuwol

Developer
Verified and confirmed.  So that everyone knows what's going on and to know what to look out for when reading through script:

When I was adding in the secondary echo stream, I changed the "print" function to indirectly call the CLI "echo" command rather than the display updater directly; this way, there would only be one channel for all prints, which would make things much easier to maintain.  Again for easier maintenance, the wrapper for all CLI routines looks for semicolons to deconstruct multiple commands on a single line and execute them in sequence.   This is where the documented security exploit comes into play.

The security exploit takes the following form.  In any function, you may see one of the following constructs:

print( visit_url( ... ) );
print( ... + visit_url( ... ) );

Now, if the string returned by visit_url() contains a semicolon, KoLmafia will, based on the wrapper routine, deconstruct it and treat it as though it were multiple commands (the first command being an echo).  The first part before the semicolon is treated as something to print.  Everything thereafter is executed as though it were a CLI command, complete with parameters, etc.  Therefore, if the offending URL linked outside of KoL, and the URL contained an embedded script somewhere that, say, sent all your meat to someone, KoLmafia would run the embedded script and send all your meat to that person.

The security risk is mild, however, as someone could just as easily write the following script, which will do something similar but without having to be devious about how they conceal their script (and it's a construct which one should be suspicious of anyway):

cli_execute( visit_url( ... ) );
cli_execute( ... + visit_url( ... ) );

Given the relative level of maturity in this forum, I trust that the people here would never, ever run or write a script like that.  As such, I don't consider this to be a deadly security risk, simply because the people here are mature enough not to do stupid stuff like take advantage of other forum members.

If you'd like to see whether or not your script is okay simply run a search for "visit_url" and make sure that the URLs which are accessed are not suspicious.  I am not sure how to address this issue.  However, because I can see why printing the results of a page are useful for the debugging process, the security exploit, which also doubles as a real bug, has been fixed and will be available in the next release.
 

Tirian

Member
It strikes me that the ability of visit_url() (and file_to_map()) to access other sites is the sort of thing that can cause mischief at a far more elementary level. I don't have the means to test it, but I've got to wonder if a black hat couldn't use url_encode() to get a string that contains the user's password and then pass that string (and my_name()) as a form parameter to a website under his control.
 
This data execution bug is way more serious than what is being seen.

If I can write to files on your computer, I can gain control of your computer.

Examples of such files:
autoexec.bat, or almost any .bat file that gets ran.
autorun.inf, and many other .inf files
many many other files on a computer can lead to a program which is already on your computer being started with commands that lead to an attacker gaining complete control of your computer. In the example given to Holatuwol, I only showed the ability to cause kolmafia to send items in a k-mail to another user.

Here is a simple script which makes a change to your computers settings. It will only work with kolmafia-9.0.exe The exe version only. It does so by modifying desktop.ini in the folder where kolmafia-9.0.exe is located, and all it does is change that folders icon to kolmafia's icon. Desktop.ini has capabilities way beyond this though. Printing a visit_url string could be used to do the same things that this script implicitly does, and more.

I am glad this is fixed in the next release.

There are many many users of kolmafia that would use it for bad things if they could. They may or may not be active in this community, but they are out there. The reason I did not give full details of the security issue in my first post is because if the info became public, then it could be exploited.
 

Attachments

  • icon.ash
    147 bytes · Views: 69

holatuwol

Developer
Tirian said:
I don't have the means to test it, but I've got to wonder if a black hat couldn't use url_encode() to get a string that contains the user's password and then pass that string (and my_name()) as a form parameter to a website under his control.

Holy ... I never thought there was a way to determine a player's password from within the ASH, but after thinking things through for a bit, there actually is, and it's been around for literally months.  That's rather scary.  This has also been fixed and will be available in the next release.

What I'm noticing based on user feedback is that the benefits of being able to access external files (like, say, the Noblesse Oblige calendar files) or submit form data (participate in large spading projects) are rather negligible.  I haven't seen a single script which uses it, or a single script which has any intention of using it; just a discussion of how it'd be "nice" to have.

Unfortunately, thanks to things like Rails which don't even require form data (just a flat URL is enough, as it transforms the URL into applicable form data), denying remote form submission alone won't fix any relevant security issues.  As such, remote URLs will be disabled from maps loading and visit_url in the next release.


efilnikufecin said:
This data execution bug is way more serious than what is being seen.  If I can write to files on your computer, I can gain control of your computer.

What you're currently documenting is an exploit of the "mirror" command, as file writing is a by-product of "mirror", not "visit_url".  I've gone ahead and added a fix for this as well, so that KoLmafia forces .txt extensions on any mirror file locations. Combined with a weakening of visit_url, I'm hoping any applicable security issues are resolved.
 
Top