Bug - Not A Bug Map_to_file directory problem

Tpj_4000

Member
It's been a while since I have been writing .ash files but I found myself with some free time and have been back at it. A long time ago (can't remember the version) mafia used to make a directory if it didn't exist when writing a file. I'm assuming it just used the default file creation call of whatever OS you are running on. However, it doesn't seem to work anymore (same OS) and always puts the files in the \data directory.

Code:
void main(){
	
	string charPath = "characters\\" + my_name() + "\\";

	string file = charPath+"test.txt";
	print("File: "+file);

	int[string] junk;
	junk["test"] = 100;

	map_to_file(junk,file);
}
 

Tpj_4000

Member
Ok, but why can't I make subdirectories inside the /data directory? I don't get how that makes anything 'secure' ?
 

jasonharper

Developer
What Bale said. It was never intended for you to be able to specify a directory, but the check to prevent that was only looking for forward slashes, not backslashes.
 

Tpj_4000

Member
What Bale said. It was never intended for you to be able to specify a directory, but the check to prevent that was only looking for forward slashes, not backslashes.

I really loathe having 90 files in the same directory. I have .ash files that generate files based on character accomplishments, alternate food and booze files, hermit clover history (for giggles), hit rates, drop statistics, etc etc. I had them broken down into organizational directories.


I understand what you are saying. I just don't get why that makes anything secure?
 

jasonharper

Developer
If you were allowed to use path separators in map filenames, scripts could access files entirely outside of KoLmafia's folder structure - perhaps overwriting something important, perhaps even accessing private information if the file format happens to be suitable for reading into a map.
 

Tpj_4000

Member
Ah ok.

You could string compare with mafiaroot\data\ and default to mafiaroot\data if not found or len(filename) < len(mafiaRootPath) (or however you want to do it), but, that's on your end if you want to do that or not. Would this be possible to add? Maybe it's better to just leave it at the data directory.
 

StDoodle

Minion
I would completely support a feature request for sub-directory access on map_to_file() & file_to_map() if it were possible to implement in such a way as to not allow other directory access in a sane way.
 

Bale

Minion
map_to_file() & file_to_map() can access sub-directories, just not explicitly. If there are such files in sub-directories then mafia will detect and use them.
 

Tpj_4000

Member
The issue is creating the subdirectory on the fly if it doesn't exist, but moreover, the ability to create files in the subdirectories. Sometimes I go through and clean house, and I don't want to go back and create a complex directory tree by hand. I had this all scripted in .ash :)

Code:
Pseudo:

if (file_exists){
     open_file
}
else{
     create_file
}

Where fight statistics went into folder "Fight Statistics", and had subfolders generated and named according to the mainhand weapon. It was pretty slick.
 

slyz

Developer
Maybe you could prefix the files with "FightStatistic_" and so on. That way you won't have folders, but that one data folder WILL be neatly organized.
 

Tpj_4000

Member
I would completely support a feature request for sub-directory access on map_to_file() & file_to_map() if it were possible to implement in such a way as to not allow other directory access in a sane way.

Also, in thinking, there would be a super simple way to do this.

boolean map_to_file( aggregate map_to_save , string file_to_save , boolean compact )

When calling map_to_file, all you have to do is append file_to_save to the mafiaRoot\data\ string and let the OS function call sort it out. If the filename and/or directory structure contained in file_to_save is bogus, map_to_file should just return false.

This way ensures that everything is in the mafiaRoot\data directory. If the user tries something like, map_to_file(myMap, "C:\windows\system32\sysoc.inf") the function will just try and create "mafiaRoot\data\C:\windows\system32\sysoc.inf" and return an error because the OS would reject that as invalid.


Maybe you could prefix the files with "FightStatistic_" and so on. That way you won't have folders, but that one data folder WILL be neatly organized.

I could, and that's the band-aid I have now. But, that sucks and like I said earlier, I loathe 90 files in one directory :)
 
Last edited:

heeheehee

Developer
Staff member
The exploit that the devs are probably concerned about is the whole "mafiaRoot/data/../../windows/system32/sysoc.inf" -- and I don't think there's an easy way to get around this.

Edit: Is that how it works at the moment?
 

Tpj_4000

Member
The exploit that the devs are probably concerned about is the whole "mafiaRoot/data/../../windows/system32/sysoc.inf" -- and I don't think there's an easy way to get around this.

Edit: Is that how it works at the moment?


StringReplace(fullFilePath, "..", "?") or something of the like. The ? character is obviously invalid for windows, and will return an error.

Why throw sticks in front of their rollerblades like that? [Dave Chappelle] 'Cause f@%! 'em. That's why [/Dave Chappelle]. Seriously though, ".." is a deliberate attempt to circumvent the security, so I'd just trigger a fail right there. I wouldn't be a nice guy and try to split the file string by the "\" character and pull out the file name and still create it for them; but that's just me :)

In any case, it's up to how the devs feel. This is more cosmetic than functional in terms of mafia's purpose.
 

xKiv

Active member
The ? character is obviously invalid for windows, and will return an error.

It isn't invalid on (some) other systems.
Also, you then might have potential issues with links/aliases and other ways to break out of a directory ... unless (SE) java can guarantee some sort of sandbox environment?
 

StDoodle

Minion
Though I'm usually not big on "hacked" solutions, I think the following of said type might be appropriate for this particular issue: Simply allow mafia to look one directory under "data" and if it finds a directory that matches the start of the file string (plus any of the various directory delimiters) in either map_to_file() or file_to_map(), allow it to be used. While I'd ideally like support for going as further "deep" into the directory as possible, just having that one level of organization would take care of 99% of the problem for 10% of the effort.
 

StDoodle

Minion
Data data everywhere, and not a... wait, what?

That is to say, I'd LOVE if this issue could be reconsidered. Even if, to simplify things, only alphanumeric characters can be used for the sub-directory name (with everything else getting stripped). That would, to quote myself:

... take care of 99% of the problem for 10% of the effort.

I even promise to add a note to the wiki that we should never, EVER request "greater flexibility" in specifying sub-directories. ;)
 
Top