Proof of Concept: Self updating scripts

Paragon

Member
Script writers can upload their script via
http://paragon.net84.net/Kol/src/upload.php


Then, users can download the script via
void main(string scriptName)
{
if (!scriptName.contains_text(".ash")) scriptName=scriptName+".ash";
string[string] some;
file_to_map("http://www.paragon.net84.net/Kol/src/"+scriptName,some);
map_to_file(some,"../scripts/"+scriptName);
}

In theory, scripts can call a function like this whenever an update is detected by zlib, and auto magically get their newest version.

It works by replacing the first newline with a tab character, and removing all other tab charaacters from the file, it also converts all //comments into /*comments*/ so that the script becomes one line.

I havn't tested it on all scripts but I have tested it on some non-trival scripts, feedback, and/or testing would be appreciated.


There is an example file smlib (badly named script to show which small medium drinks you have in inventory.) If you create a .ash file named auto.ash with above code, you can test it by
Cli commands:
auto smlib
smlib
 
Last edited:

Paragon

Member
One other thing I should mention, If you do test it with one of your own scripts, make sure you save a second original copy b/c the file you get back will be a one line script and it will be difficult to reformat the code to something readable.
 

fronobulax

Developer
Staff member
While I have not tried your script, I have had problems with self updating scripts in the past because there is no guarantee that the operating system will truly release the file handle to the running copy in time for the update to overwrite when it arrives: the alternative being for the overwrite to fail because the original still has an open file handle. I believe mafia reads in the script before running so mafia will not be keeping an open file handle but it will still be a manual process (as it is now) to rerun after the update. Not a huge deal but most things that I use that auto update (and are useful to me) either can update "on the fly" or offer an automated restart option.
 

Paragon

Member
I am not a java guy, but from looking at mafias code i *think* mafia loads a script into memory, then when it goes to run that script, it checks if the file has been updated, if it has been updated it will reload the file into memory. This should mean that the file is free to be written to when the script is running.

The way it would work, would be 1) you run a script, if the script notices that a new version is available, it downloads the new script, it completes as the current outdated script, then the next time you run the script the new version is used.

I think it would be possible that if the script detects that a new version is neccessary, it downloads the new version, then starts a new instance of the script, then once the new version is complete the instance that spawns it exits without anything further happing. But if I remember correctly, even though it has worked when tested in the past, It is frowned upon to instantiate a script from another script.

So, while it would be a manual effort to rerun the script, the act of finding and downloading the script would no longer be manual.

Ideally, it would be incorporated into zlibs version checking code such that instead of getting a message that says "New version available, download from xyz" it would say "Script updated, changes will take effect next time this script is run", granted all the scripts that currently use zlibs version checking function would need to be altered to use a overloaded version of the version checking function (we wouldn't want to screw up scripts that havn't been uploaded to the auto update repository) and we would need to do one last manual update for all the scripts that currently exist that do not use the (potential) new version of the zlib version checking but after that, and for scripts from here into the future it would be easy to stay up to date.
Another method, that would not require a manual update to all scripts but only to zlib, would be that zlib check if the default repository contains the script, auto updating if it is found and acting as it currently does otherwise.

I am also open to sharing the upload php script to anyone so even the repository that the script is downloaded from could be controlled by the end scripter.
 
Last edited:

fronobulax

Developer
Staff member
I am not a java guy, but from looking at mafias code i *think* mafia loads a script into memory, then when it goes to run that script, it checks if the file has been updated, if it has been updated it will reload the file into memory. This should mean that the file is free to be written to when the script is running.

While I have not tried your script, I have had problems with self updating scripts in the past because there is no guarantee that the operating system will truly release the file handle to the running copy in time for the update to overwrite when it arrives: the alternative being for the overwrite to fail because the original still has an open file handle.

Your Java analysis agrees with mine but it is the operating system (Windows, *nix, etc.) that is going to cause this to be less than 100% reliable and not KoLmafia.
 

Paragon

Member
Oh, sorry... missed the distinction. Well, the only way I can think of checking that is to test it.
Soo... if anyone has a *nix machine (or mac for that matter) and would like to test this you can cut/past the code listed above then cli
auto auto
auto auto

The first run should not contain any out put.
The second run should output: Updated script: Auto.ash

(It actually updated it the first time, and really only overwrote the same code the 2nd time, but the test is still valid.)
Windows Vista is not having any trouble.
 

Catch-22

Active member
There's no file locking interference, based on the above here's what happens:

Mafia loads the script into memory from the hard drive.
Mafia executes the script in memory.
Script updates the file on the hard drive.

That's it. The script doesn't update dynamically in memory. You'd need to run it again in order for it to update the script in memory.

You'd probably need to have some sort of unique identifier that will change when the script has been updated, the ID should be embedded in the script on the remote server. If the ID has changed, the script should flag that there has been an update to the script and restart itself.
 

lostcalpolydude

Developer
Staff member
If it's relying on anything besides kolmafia.us (and maybe even if it does end up using kolmafia.us somehow), it should probably make sure that the page actually loads and isn't an error page before overwriting the script with the contents of the page.
 

Catch-22

Active member
Are we really supposed to allow this? I don't have time to check the source to see what restrictions we have in place exactly, but this feature doesn't sound very safe.

I agree, I believe in the thread discussion I linked to earlier this came up and it was mentioned that KoLmafia wouldn't allow you to write to the scripts folder. As such, I don't think it should be allowed. This means someone could easily hijack/sabotage another script by overwriting it.
 

fronobulax

Developer
Staff member
Mafia loads the script into memory from the hard drive.
Mafia executes the script in memory.
Script updates the file on the hard drive.

That's it.

Actually that's not it although I don't really care enough to figure out which operating system did this to me several times and reproduce it.

Mafia opens file.
OS flags file as open and does all its OS stuff so that mafia can access the file.
Mafia closes file.
OS does all the OS stuff it wants to do before it closes the file, possibly including waiting introduced by delayed writes, caching, lag due to hardware or anything else.
Mafia opens updated file.
OS opens file but because the previous open has not closed yet Bad Things Happen.

This is why a lot of operating systems have explicit file locking and this is why a lot of real time applications with critical I/O bypass the operating system or require specific brands of disks and versions of associated firmware.

Now a point I thought I made previously, but seems to have been missed, is that I am not saying it is likely to happen with mafia opening script files. I am, however saying, that it has happened to me enough times in the past that I would rather not use the feature as implemented than risk data loss again. YMMV.
 

Paragon

Member
Yea, I was thinking about the security issue, and obviously as it stands now it isn't entirely safe. But if I 1) Made it such that users had to register to upload their scripts to the repository, and only the original uploader could update that script then you would have to trust the script writer as much as you trust the script writer now. 2) If it was embedded into zlib, then the the scripts would only update when the original poster updated the scripts original thread. Sure, someone could modify that code, but the only person it would effect would be themselves... so someone would have to purposely put them selves at higher risk. Those two actions combied though would be the enough for me to feel fairly secure using it.

On the issue of security though, scripts like OCD could also be considered Insecure, and disallowing writing to the script directory would not solve the problem. Imagine that someone wrote a script that behind the scenes, rewrote your OCD datafile such that it told OCD to autosell, or giveaway all your stuff. OCD is a great script, and I hardly think that would happen... but it was just the first example that popped out at me for not being 100% secure.

As for the issue fronobulax is talking about, that is starting to sound alot more like an OS problem then any non-os software issues. The thing is that ash doesn't have much in the way of file handling to begin with, so we work with what we have, also, maybe some really important software is requiring specifc machine set ups, but for the most part software relies on the I/O libraries of the language that they are written in, most of which allow locking files to be opened read-only, write-only, read write synchronous and read-write asynchronously. I am certain that for any software that relies heavily on one reader/one writer at a time there are an equal number that rely heavily on allowing multiple users to read/write simultaneously. Either way though our Kol scripts aren't exactly the most critial systems on our machines. I can understand a trick me once sham on you atttitude but can you seriously tell me that you won't trust any software that writes to the same file multiple times a session
 

Catch-22

Active member
Paragon I don't think the security of your script or servers is really coming in to question (not that it shouldn't). The problem is that KoLmafia currently exposes the user to directory traversal attacks. This is unsafe and should probably not be allowed.

most of which allow locking files to be opened read-only

FileInputStream, which is what KoLmafia uses to load a script into memory, is read-only. So, I'm also baffled by the issue fronobulax faced in the past.
 

fronobulax

Developer
Staff member
For a possibly related issue that has happened with KoLmafia and does not rely on my personal paranoia or bad experience with operating systems that were probably obsolete before several posters were born (and that could include Windows ;-) ), I refer you to all of the times KoLmafia preferences and settings files were corrupted, overwritten or destroyed by multiple instances of KoLmafia running from the same directory.

The OCD example is cute. The nitpicker in me wants to note that the referenced files are in the data directory, not the scripts directory.

I concur that if there really is an opportunity for directory traversal attacks then the hole should be plugged.
 

xKiv

Active member
OS opens file but because the previous open has not closed yet Bad Things Happen.

It's worth mentioning that the OS might want to update (meta)data that most people wouldn't consider part of the file.
And also that updating it from two different handles will probably not have the file end up in an inconsistent state (from the OS's point of view), but it might end up inconsistent with other files/things.

This is why a lot of operating systems have explicit file locking and this is why a lot of real time applications with critical I/O bypass the operating system or require specific brands of disks and versions of associated firmware.

Also why overwriting file A is often approached by writing a completely different file B and then renaming B to A (which will atomically sidestep many possible problems)

Now a point I thought I made previously, but seems to have been missed, is that I am not saying it is likely to happen with mafia opening script files. I am, however saying, that it has happened to me enough times in the past that I would rather not use the feature as implemented than risk data loss again. YMMV.

Also worth noting to some people here is that (as far as I can tell) F~ is not saying that he had this happen to mafia scripts. Rather, he's saying that you can't prove (to his satisfaction) that it will never happen.
 

Theraze

Active member
Quick proof of concept:

Removed: All forum mods can see the old version of the post if they need the example. -lost
 
Last edited by a moderator:

Veracity

Developer
Staff member
Ran auto auto and my new auto.ash ended up in my Program Files menu, instead of Program Files\KoLmafia. Yep, directory traversal verified.
Thanks for publicly pointing out this security flaw. Hopefully, we will plug it soon. And then I guess I'd better spin a new release...
 

holatuwol

Developer
I probably introduced this bug when I tried to make things work a little better with symlinks in r9131. I've introduced alternate logic to handle the same problem in r11348 which should achieve the same thing, but without the security vulnerability.
 

Bale

Minion
Thanks for publicly pointing out this security flaw. Hopefully, we will plug it soon. And then I guess I'd better spin a new release...

It's a particularly nice time for a new point release since we've got new zones for the McLarge Huge quest. There's a new IotM to add also, so hopefully it will end up in the next release.
 
Top