View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0000579Spring engineGeneralpublic2007-08-09 11:372017-02-15 14:57
Reporterredstar Assigned TojK  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
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.
Attached Files
exploit.JPG (Attachment missing)
Checked infolog.txt for Errors

Activities

imbaczek

2007-08-09 14:21

reporter   ~0001058

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.

Pendrokar

2007-08-09 15:35

reporter   ~0001059

Well can lua delete files?

tvo

2007-08-09 22:01

reporter   ~0001062

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.

imbaczek

2007-08-09 22:38

reporter   ~0001063

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.

tvo

2007-08-11 20:55

reporter   ~0001079

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

imbaczek

2007-08-12 21:08

reporter   ~0001084

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?

tvo

2007-08-13 08:13

reporter   ~0001090

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).

imbaczek

2007-08-13 09:10

reporter   ~0001094

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.)

abma

2011-07-03 23:47

administrator   ~0006895

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)

hoijui

2011-07-04 12:43

reporter   ~0006901

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>

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