Skip to content

Commit

Permalink
Fix bug that permits download of arbitrary files from a download enab…
Browse files Browse the repository at this point in the history
…led server by checking requested file name against the list of loaded pk3 files. See CVE-2006-2082
  • Loading branch information
Thilo Schulz committed May 8, 2006
1 parent fda7db7 commit 60293f4
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 15 deletions.
9 changes: 1 addition & 8 deletions code/qcommon/files.c
Expand Up @@ -2556,16 +2556,9 @@ FS_idPak
*/
qboolean FS_idPak( char *pak, char *base ) {
int i;
char pakbuf[MAX_QPATH], *pakptr;

// Chop off filename extension if necessary.
Com_sprintf(pakbuf, sizeof(pakbuf), "%s", pak);
pakptr = Q_strrchr(pakbuf, '.');
if(pakptr)
*pakptr = '\0';

for (i = 0; i < NUM_ID_PAKS; i++) {
if ( !FS_FilenameCompare(pakbuf, va("%s/pak%d", base, i)) ) {
if ( !FS_FilenameCompare(pak, va("%s/pak%d", base, i)) ) {
break;
}
}
Expand Down
52 changes: 45 additions & 7 deletions code/server/sv_client.c
Expand Up @@ -756,24 +756,60 @@ void SV_WriteDownloadToClient( client_t *cl , msg_t *msg )
int curindex;
int rate;
int blockspersnap;
int idPack, missionPack;
int idPack, missionPack, unreferenced = 1;
char errorMessage[1024];
char pakbuf[MAX_QPATH], *pakptr;
int numRefPaks;

if (!*cl->downloadName)
return; // Nothing being downloaded

if (!cl->download) {
// We open the file here
// Chop off filename extension.
Com_sprintf(pakbuf, sizeof(pakbuf), "%s", cl->downloadName);
pakptr = Q_strrchr(pakbuf, '.');

if(pakptr)
{
*pakptr = '\0';

// Check for pk3 filename extension
if(!Q_stricmp(pakptr + 1, "pk3"))
{
const char *referencedPaks = FS_ReferencedPakNames();

// Check whether the file appears in the list of referenced
// paks to prevent downloading of arbitrary files.
Cmd_TokenizeStringIgnoreQuotes(referencedPaks);
numRefPaks = Cmd_Argc();

Com_Printf( "clientDownload: %d : begining \"%s\"\n", cl - svs.clients, cl->downloadName );
for(curindex = 0; curindex < numRefPaks; curindex++)
{
if(!FS_FilenameCompare(Cmd_Argv(curindex), pakbuf))
{
unreferenced = 0;

missionPack = FS_idPak(cl->downloadName, "missionpack");
idPack = missionPack || FS_idPak(cl->downloadName, BASEGAME);
// now that we know the file is referenced,
// check whether it's legal to download it.
missionPack = FS_idPak(pakbuf, "missionpack");
idPack = missionPack || FS_idPak(pakbuf, BASEGAME);

break;
}
}
}
}

if ( !sv_allowDownload->integer || idPack ||
// We open the file here
if ( !sv_allowDownload->integer || idPack || unreferenced ||
( cl->downloadSize = FS_SV_FOpenFileRead( cl->downloadName, &cl->download ) ) <= 0 ) {
// cannot auto-download file
if (idPack) {
if(unreferenced)
{
Com_Printf("clientDownload: %d : \"%s\" is not referenced and cannot be downloaded.\n", cl - svs.clients, cl->downloadName);
Com_sprintf(errorMessage, sizeof(errorMessage), "File \"%s\" is not referenced and cannot be downloaded.", cl->downloadName);
}
else if (idPack) {
Com_Printf("clientDownload: %d : \"%s\" cannot download id pk3 files\n", cl - svs.clients, cl->downloadName);
if (missionPack) {
Com_sprintf(errorMessage, sizeof(errorMessage), "Cannot autodownload Team Arena file \"%s\"\n"
Expand Down Expand Up @@ -809,6 +845,8 @@ void SV_WriteDownloadToClient( client_t *cl , msg_t *msg )
return;
}

Com_Printf( "clientDownload: %d : beginning \"%s\"\n", cl - svs.clients, cl->downloadName );

// Init
cl->downloadCurrentBlock = cl->downloadClientBlock = cl->downloadXmitBlock = 0;
cl->downloadCount = 0;
Expand Down

0 comments on commit 60293f4

Please sign in to comment.