2019-08-24 00:22 CEST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0000579Spring engineGeneralpublic2017-02-15 14:57
Reporterredstar 
Assigned TojK 
PrioritynormalSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
Product Version 
Target VersionFixed in Version 
Summary0000579: Security Exploit: Retrieving critical system files and directory listings via VFS
DescriptionLua scripts, and programs using unitsync can be exploited to list folders outside the spring resource folders.
Steps To ReproduceThe following code was added to the CBattleWindow class udner the HostGame function:

String sk = CUnitSyncJNIBindings.SearchVFS ("C:/Windows/*");
this.AIEditPane.setText (sk.replaceAll (",","\n "));

A game was then started, and the AI tab selected, the results are attached in a screenshot.
TagsNo tags attached.
Checked infolog.txt for lua Errors
Attached Files

-Relationships
+Relationships

-Notes

~0001058

imbaczek (reporter)

I agree that it shouldn't happen, but IMHO as long as it's not possible to invoke this remotely, that's not an exploit.

~0001059

Pendrokar (reporter)

Well can lua delete files?

~0001062

tvo (reporter)

It's still a bug.

IIRC I explicitly blocked this in the Linux part of the VFS, I think it searches for patterns like ../ and resolves those by itself unless they point outside the VFS, in which case it returns an error or ignores them. (So it works a bit like a chroot jail.)

I think this should be fixed with a next major rewrite of the filesystem code though, which should happen anyway to reduce platform specific code, add multiple data directories on windows, fix redundant stat()'ing and access()'ing files and fix the expensive recursive search for archives that makes running Spring with big .sdd archives slow.

~0001063

imbaczek (reporter)

there's physfs http://icculus.org/physfs/ and from what I see it does everything a VFS layer should do. The less code in spring, the better IMHO, and there's no need to reinvent the wheel.

~0001079

tvo (reporter)

From the documentation that seems like the perfect library.
If you feel like some refactor, go ahead.

~0001084

imbaczek (reporter)

I'm investigating the issue. Physfs doesn't support old TA files, but I haven't seen any mod that uses them; theoretically the current support could stay, but I don't see the point. Could we drop them?

~0001090

tvo (reporter)

Just assume we could drop them for now. I doubt many people still use them, since the support in Spring for TA files is quite bad already (ie. they don't have modinfo.tdf, if they are in root of data folder it breaks multiplayer, etc.)

If we really need to keep them we could always resort to patching Physfs.

Another thing is the lack of case insensitivity. I think this can be easily added on top of Physfs though, by having it list all files in all directories recursively and putting that in a std::map<std::string, std::string> that maps lowercase filenames to mixed case filenames (see ArchiveDir.cpp for example).

~0001094

imbaczek (reporter)

my (limited) testing has shown that physfs is case insensitive. (at least the test program that gets build alongside the library prints all mounted files as if they were lowercase.)

~0006895

abma (administrator)

Last edited: 2011-07-04 00:11

does this still work? (yes i know, this report is years old... but it was assigned and never closed)

~0006901

hoijui (reporter)

yes, this exploit is still possible in spring 0.82.3-2454-g5bf7d23,
though the code for testing changed a bit:
<code>
//String toListGlob = "C:/Windows/*";
const char* toListGlob = "/home/userX/*";
printf("Listing contents of VFS: %s", toListGlob);
int err = unitsync.InitFindVFS(toListGlob);
if (err == 0) {
    int fileId = 0;
    int nameBuffer_size = 1024;
    char* nameBuffer = new char[nameBuffer_size];
    while (true) {
        fileId = unitsync.FindFilesVFS(fileId, nameBuffer, nameBuffer_size);
        if (fileId > 0) {
            printf("\t%s", nameBuffer);
        } else {
            break;
        }
    }
}
</code>
+Notes

-Issue History
Date Modified Username Field Change
2007-08-09 11:37 redstar New Issue
2007-08-09 11:37 redstar File Added: exploit.JPG
2007-08-09 14:21 imbaczek Note Added: 0001058
2007-08-09 15:35 Pendrokar Note Added: 0001059
2007-08-09 22:01 tvo Note Added: 0001062
2007-08-09 22:38 imbaczek Note Added: 0001063
2007-08-11 20:55 tvo Note Added: 0001079
2007-08-12 21:08 imbaczek Note Added: 0001084
2007-08-13 08:13 tvo Note Added: 0001090
2007-08-13 09:10 imbaczek Note Added: 0001094
2009-07-26 10:22 hoijui Status new => assigned
2009-07-26 10:22 hoijui Assigned To => hoijui
2009-07-26 10:23 hoijui Assigned To hoijui => imbaczek
2011-07-03 23:46 abma Assigned To imbaczek =>
2011-07-03 23:47 abma Status assigned => new
2011-07-03 23:47 abma Note Added: 0006895
2011-07-03 23:47 abma Status new => feedback
2011-07-04 00:11 abma Note Edited: 0006895
2011-07-04 12:43 hoijui Note Added: 0006901
2011-07-04 12:44 hoijui Status feedback => confirmed
2014-02-03 15:02 jK Status confirmed => resolved
2014-02-03 15:02 jK Resolution open => fixed
2014-02-03 15:02 jK Assigned To => jK
+Issue History