From 60293f49ee8c665673202e80ecd103f13a9fa6ab Mon Sep 17 00:00:00 2001 From: Thilo Schulz Date: Mon, 8 May 2006 19:53:41 +0000 Subject: [PATCH] Fix bug that permits download of arbitrary files from a download enabled server by checking requested file name against the list of loaded pk3 files. See CVE-2006-2082 --- code/qcommon/files.c | 9 +------ code/server/sv_client.c | 56 ++++++++++++++++++++++++++++++++++------- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/code/qcommon/files.c b/code/qcommon/files.c index 13b8a257..31b9b663 100644 --- a/code/qcommon/files.c +++ b/code/qcommon/files.c @@ -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; } } diff --git a/code/server/sv_client.c b/code/server/sv_client.c index 1277c1d0..c170866b 100644 --- a/code/server/sv_client.c +++ b/code/server/sv_client.c @@ -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) { + // 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(); + + for(curindex = 0; curindex < numRefPaks; curindex++) + { + if(!FS_FilenameCompare(Cmd_Argv(curindex), pakbuf)) + { + unreferenced = 0; + + // 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; + } + } + } + } + // We open the file here - - Com_Printf( "clientDownload: %d : begining \"%s\"\n", cl - svs.clients, cl->downloadName ); - - missionPack = FS_idPak(cl->downloadName, "missionpack"); - idPack = missionPack || FS_idPak(cl->downloadName, BASEGAME); - - if ( !sv_allowDownload->integer || idPack || + 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" @@ -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;