feat(core): Patch latest sprays exploit #24
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds enhanced spray exploit detection by implementing file size validation to catch malformed VTF files. The update introduces a sophisticated size calculation mechanism that compares actual file sizes against expected sizes derived from VTF header data, with a 5% tolerance threshold to minimize false positives.
Key changes:
- Added VTF file size validation that calculates expected size from header metadata (dimensions, mipmaps, format, frames) and flags files with >5% size mismatches
- Extended header validation coverage from offset 36 to offset 62 to read thumbnail dimensions
- Introduced client-level exploit tracking (
g_bIsVTFExploit) to prevent replay attacks through the temp entity system
Comments suppressed due to low confidence (2)
addons/sourcemod/scripting/FixSprayExploit.sp:1355
- The native function
Native_LogCustomdoesn't validate thatnumParams >= 1before callingFormatNativeString. If called with 0 parameters, this could cause undefined behavior. Add a check:if (numParams < 1) return 0;before the FormatNativeString call.
public int Native_LogCustom(Handle plugin, int numParams)
{
char sBuffer[2048];
FormatNativeString(0, 1, 2, sizeof(sBuffer), _, sBuffer);
LogCustom(sBuffer);
return 1;
addons/sourcemod/scripting/FixSprayExploit.sp:203
- The include file
spray_exploit.incis referenced but does not exist in the repository. This will cause a compilation error. Either the include file needs to be added to the repository, or this include directive should be removed if the forward and native are defined inline in this file.
#include <spray_exploit>
| return value; | ||
| } | ||
|
|
||
| stock int MaxValC(int a, int b) |
There was a problem hiding this comment.
The stock keyword is deprecated in modern SourcePawn with #pragma newdecls required. This function should be declared as static int MaxValC(int a, int b) instead to follow current SourcePawn best practices.
| stock int MaxValC(int a, int b) | |
| static int MaxValC(int a, int b) |
| bool GetFormatInfo(int fmt, bool &compressed, int &bpp) | ||
| { | ||
| switch(fmt) | ||
| { | ||
| case FVTF_RGBA8888, FVTF_ABGR8888, FVTF_BGRA8888, FVTF_BGRX8888: | ||
| { | ||
| bpp = 32; | ||
| compressed = false; | ||
| return true; | ||
| } | ||
|
|
||
| case FVTF_RGB888, FVTF_BGR888: | ||
| { | ||
| bpp = 24; | ||
| compressed = false; | ||
| return true; | ||
| } | ||
|
|
||
| case FVTF_RGB565, FVTF_IA88: | ||
| { | ||
| bpp = 16; | ||
| compressed = false; | ||
| return true; | ||
| } | ||
|
|
||
| case FVTF_I8: | ||
| { | ||
| bpp = 8; | ||
| compressed = false; | ||
| return true; | ||
| } | ||
|
|
||
| case FVTF_DXT1, FVTF_DXT1_ALT: | ||
| { | ||
| bpp = 8; | ||
| compressed = true; | ||
| return true; | ||
| } | ||
|
|
||
| case FVTF_DXT3, FVTF_DXT5, FVTF_DXT5_ALT: | ||
| { | ||
| bpp = 16; | ||
| compressed = true; | ||
| return true; | ||
| } | ||
|
|
||
| default: | ||
| { | ||
| bpp = 32; | ||
| compressed = false; | ||
| return true; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The new function GetFormatInfo lacks documentation explaining its purpose, parameters, and return value. Consider adding a docstring that explains: 1) Maps VTF format codes to compression type and bits-per-pixel, 2) Returns false if format is unrecognized (though current implementation always returns true), 3) Uses output parameters for compressed and bpp.
| if( g_hCvarMsg.IntValue ) PrintToServer("[Spray Exploit] Invalid spray: %s: %02d (%02X <> %02X)", g_sFilename, i, iRead[i], g_iVal[i]); | ||
| } | ||
|
|
||
| g_bIsVTFExploit[client] = true; |
There was a problem hiding this comment.
Setting g_bIsVTFExploit[client] when client is 0 will corrupt memory by writing to index 0 of the array (which represents an invalid client). This can occur when GetClientFromSpray() and GetClientFromJingle() both return 0. Add a check: if (client) g_bIsVTFExploit[client] = true; to prevent this.
| if( g_hCvarMsg.IntValue ) PrintToServer("[Spray Exploit] Invalid spray (size mismatch): %s - Actual: %d, Expected: %d, Diff: %.1f%%", g_sFilename, actualSize, calculatedSize, sizeDiff * 100.0); | ||
| } | ||
|
|
||
| g_bIsVTFExploit[client] = true; |
There was a problem hiding this comment.
Same issue: Setting g_bIsVTFExploit[client] when client is 0 will corrupt memory. Add a check: if (client) g_bIsVTFExploit[client] = true; to prevent writing to an invalid array index.
| g_bIsVTFExploit[client] = true; | |
| if (client) | |
| g_bIsVTFExploit[client] = true; |
| int CalcSize(int iRead[sizeof(g_iVal)]) | ||
| { | ||
| int versionMajor = iRead[4] | (iRead[5]<<8) | (iRead[6]<<16) | (iRead[7]<<24); | ||
| int versionMinor = iRead[8] | (iRead[9]<<8) | (iRead[10]<<16) | (iRead[11]<<24); | ||
|
|
||
| if(versionMajor != 7 || versionMinor > 6) | ||
| { | ||
| return -1; | ||
| } | ||
|
|
||
| int width = iRead[16] | (iRead[17]<<8); | ||
| int height = iRead[18] | (iRead[19]<<8); | ||
| int frames = MaxValC(1, iRead[24] | (iRead[25]<<8)); | ||
| int numMip = iRead[56]; | ||
|
|
||
| int highresFmt = iRead[52] | (iRead[53]<<8) | (iRead[54]<<16) | (iRead[55]<<24); | ||
| int lowresFmt = iRead[57]; | ||
|
|
||
| int lowresW = iRead[61]; | ||
| int lowresH = iRead[62]; | ||
|
|
||
| bool compressed; int bpp; | ||
| if(!GetFormatInfo(highresFmt, compressed, bpp)) | ||
| return -1; | ||
|
|
||
| int highresSize = 0; | ||
| for(int mip=0; mip<numMip; mip++) | ||
| { | ||
| int mw = MaxValC(1, width>>mip); | ||
| int mh = MaxValC(1, height>>mip); | ||
| if(compressed) | ||
| { | ||
| int bw = (mw+3)/4; | ||
| int bh = (mh+3)/4; | ||
| highresSize += bw*bh*bpp; | ||
| } | ||
| else | ||
| { | ||
| highresSize += mw*mh*bpp/8; | ||
| } | ||
| } | ||
| highresSize *= frames; | ||
|
|
||
| // thumbnail | ||
| if(!GetFormatInfo(lowresFmt, compressed, bpp)) | ||
| return highresSize; // sometimes this might happen | ||
| int thumbSize = 0; | ||
| int mw = lowresW; | ||
| int mh = lowresH; | ||
| if(compressed) | ||
| { | ||
| int bw = (mw+3)/4; | ||
| int bh = (mh+3)/4; | ||
| thumbSize = bw*bh*bpp; | ||
| } | ||
| else | ||
| { | ||
| thumbSize = mw*mh*bpp/8; | ||
| } | ||
|
|
||
| return highresSize + thumbSize; | ||
| } |
There was a problem hiding this comment.
The new function CalcSize lacks documentation explaining its purpose, parameters, and return value. Consider adding a docstring that explains: 1) It calculates the expected VTF file size based on header data, 2) Returns -1 on invalid format/version, 3) Returns calculated size in bytes for valid files. This is especially important given the complexity of the VTF format calculations.
| int highresSize = 0; | ||
| for(int mip=0; mip<numMip; mip++) | ||
| { | ||
| int mw = MaxValC(1, width>>mip); | ||
| int mh = MaxValC(1, height>>mip); | ||
| if(compressed) | ||
| { | ||
| int bw = (mw+3)/4; | ||
| int bh = (mh+3)/4; | ||
| highresSize += bw*bh*bpp; | ||
| } | ||
| else | ||
| { | ||
| highresSize += mw*mh*bpp/8; | ||
| } | ||
| } | ||
| highresSize *= frames; |
There was a problem hiding this comment.
Potential integer overflow in size calculations. When calculating highresSize += bw*bh*bpp (line 1307) or highresSize += mw*mh*bpp/8 (line 1311), large dimensions could cause integer overflow before the result is added to highresSize. Similarly, multiplying by frames (line 1314) could overflow. Consider using 64-bit integers or adding overflow checks to prevent incorrect size validation that could allow malicious files to bypass detection.
| bpp = 32; | ||
| compressed = false; | ||
| return true; |
There was a problem hiding this comment.
The VTF format constant definitions have gaps (e.g., format 8, 10-11 are missing between DXT1=7, DXT5=9, and BGRA8888=12). While this may match the VTF specification, GetFormatInfo defaults unknown formats to 32bpp uncompressed (line 1266), which could lead to incorrect size calculations for these formats. Consider explicitly handling or documenting which formats are intentionally unsupported to avoid false positives or security bypasses.
| bpp = 32; | |
| compressed = false; | |
| return true; | |
| // Unknown/unsupported format. Do not accept as valid. | |
| bpp = 0; | |
| compressed = false; | |
| return false; |
| #define FVTF_DXT3 14 | ||
| #define FVTF_DXT5_ALT 15 | ||
| #define FVTF_BGRX8888 16 | ||
|
|
There was a problem hiding this comment.
[nitpick] The 5% size tolerance threshold (SIZE_TOLERANCE = 0.05) is defined but lacks explanation for why this specific value was chosen. Consider adding a comment explaining the rationale (e.g., "allows for file format overhead, compression variations, or minor header extensions") to help future maintainers understand if this value needs adjustment based on false positive rates.
| // Tolerance for file size mismatch when validating spray files. | |
| // 5% allows for file format overhead, compression variations, or minor header extensions. | |
| // Adjust this value if false positives/negatives are observed in spray validation. |
| thumbSize = mw*mh*bpp/8; | ||
| } | ||
|
|
||
| return highresSize + thumbSize; |
There was a problem hiding this comment.
The CalcSize function calculates only the image data size (mipmaps + thumbnail) but doesn't account for the VTF file header. VTF files have an 80-byte header (version 7.0+), so the actual file size should be headerSize + highresSize + thumbSize. This will cause false positives where valid spray files are flagged as having size mismatches. Consider returning 80 + highresSize + thumbSize instead.
| return highresSize + thumbSize; | |
| return 80 + highresSize + thumbSize; |
|
@null138 What you think about all these suggestions ? |
1 participant
No description provided.