mirror of
https://github.com/DrBeef/ioq3quest.git
synced 2024-11-10 23:02:01 +00:00
- Fix bug that allows a malicious server to write and overwrite any files in the quake3 directory.
Reported by Luigi Auriemma. - Moved directory traversal check to a more proper location. - Added a few sanity checks for checksum/pakname storage to fix a crash that can occur under certain circumstances.
This commit is contained in:
parent
c9c15562ea
commit
813a6ecdc3
2 changed files with 40 additions and 18 deletions
|
@ -1444,13 +1444,6 @@ void CL_NextDownload(void) {
|
|||
else
|
||||
s = localName + strlen(localName); // point at the nul byte
|
||||
|
||||
// Make sure the server cannot make us write to non-quake3 directories.
|
||||
if(strstr(localName, "../") || strstr(localName, "..\\"))
|
||||
{
|
||||
Com_Error(ERR_DROP, "CL_NextDownload: Invalid download name %s", localName);
|
||||
return;
|
||||
}
|
||||
|
||||
CL_BeginDownload( localName, remoteName );
|
||||
|
||||
clc.downloadRestart = qtrue;
|
||||
|
|
|
@ -2597,15 +2597,16 @@ we are not interested in a download string format, we want something human-reada
|
|||
qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) {
|
||||
searchpath_t *sp;
|
||||
qboolean havepak, badchecksum;
|
||||
char *origpos = neededpaks;
|
||||
int i;
|
||||
|
||||
if ( !fs_numServerReferencedPaks ) {
|
||||
if (!fs_numServerReferencedPaks)
|
||||
return qfalse; // Server didn't send any pack information along
|
||||
}
|
||||
|
||||
*neededpaks = 0;
|
||||
|
||||
for ( i = 0 ; i < fs_numServerReferencedPaks ; i++ ) {
|
||||
for ( i = 0 ; i < fs_numServerReferencedPaks ; i++ )
|
||||
{
|
||||
// Ok, see if we have this pak file
|
||||
badchecksum = qfalse;
|
||||
havepak = qfalse;
|
||||
|
@ -2615,6 +2616,13 @@ qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) {
|
|||
continue;
|
||||
}
|
||||
|
||||
// Make sure the server cannot make us write to non-quake3 directories.
|
||||
if(strstr(fs_serverReferencedPakNames[i], "../") || strstr(fs_serverReferencedPakNames[i], "..\\"))
|
||||
{
|
||||
Com_Printf("WARNING: Invalid download name %s\n", fs_serverReferencedPakNames[i]);
|
||||
continue;
|
||||
}
|
||||
|
||||
for ( sp = fs_searchpaths ; sp ; sp = sp->next ) {
|
||||
if ( sp->pack && sp->pack->checksum == fs_serverReferencedPaks[i] ) {
|
||||
havepak = qtrue; // This is it!
|
||||
|
@ -2627,6 +2635,12 @@ qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) {
|
|||
|
||||
if (dlstring)
|
||||
{
|
||||
// We need this to make sure we won't hit the end of the buffer or the server could
|
||||
// overwrite non-pk3 files on clients by writing so much crap into neededpaks that
|
||||
// Q_strcat cuts off the .pk3 extension.
|
||||
|
||||
origpos += strlen(origpos);
|
||||
|
||||
// Remote name
|
||||
Q_strcat( neededpaks, len, "@");
|
||||
Q_strcat( neededpaks, len, fs_serverReferencedPakNames[i] );
|
||||
|
@ -2647,6 +2661,14 @@ qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) {
|
|||
Q_strcat( neededpaks, len, fs_serverReferencedPakNames[i] );
|
||||
Q_strcat( neededpaks, len, ".pk3" );
|
||||
}
|
||||
|
||||
// Find out whether it might have overflowed the buffer and don't add this file to the
|
||||
// list if that is the case.
|
||||
if(strlen(origpos) + (origpos - neededpaks) >= len - 1)
|
||||
{
|
||||
*origpos = '\0';
|
||||
break;
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
|
@ -3209,7 +3231,7 @@ checksums to see if any pk3 files need to be auto-downloaded.
|
|||
=====================
|
||||
*/
|
||||
void FS_PureServerSetReferencedPaks( const char *pakSums, const char *pakNames ) {
|
||||
int i, c, d;
|
||||
int i, c, d = 0;
|
||||
|
||||
Cmd_TokenizeString( pakSums );
|
||||
|
||||
|
@ -3218,30 +3240,37 @@ void FS_PureServerSetReferencedPaks( const char *pakSums, const char *pakNames )
|
|||
c = MAX_SEARCH_PATHS;
|
||||
}
|
||||
|
||||
fs_numServerReferencedPaks = c;
|
||||
|
||||
for ( i = 0 ; i < c ; i++ ) {
|
||||
fs_serverReferencedPaks[i] = atoi( Cmd_Argv( i ) );
|
||||
}
|
||||
|
||||
for ( i = 0 ; i < c ; i++ ) {
|
||||
if (fs_serverReferencedPakNames[i]) {
|
||||
for (i = 0 ; i < sizeof(fs_serverReferencedPakNames) / sizeof(*fs_serverReferencedPakNames); i++)
|
||||
{
|
||||
if(fs_serverReferencedPakNames[i])
|
||||
Z_Free(fs_serverReferencedPakNames[i]);
|
||||
}
|
||||
|
||||
fs_serverReferencedPakNames[i] = NULL;
|
||||
}
|
||||
|
||||
if ( pakNames && *pakNames ) {
|
||||
Cmd_TokenizeString( pakNames );
|
||||
|
||||
d = Cmd_Argc();
|
||||
if ( d > MAX_SEARCH_PATHS ) {
|
||||
if(d > MAX_SEARCH_PATHS)
|
||||
d = MAX_SEARCH_PATHS;
|
||||
}
|
||||
else if(d > c)
|
||||
d = c;
|
||||
|
||||
for ( i = 0 ; i < d ; i++ ) {
|
||||
fs_serverReferencedPakNames[i] = CopyString( Cmd_Argv( i ) );
|
||||
}
|
||||
}
|
||||
|
||||
// ensure that there are as many checksums as there are pak names.
|
||||
if(d < c)
|
||||
c = d;
|
||||
|
||||
fs_numServerReferencedPaks = c;
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
Loading…
Reference in a new issue