Uh oh!
There was an error while loading. Please reload this page.
fix: original msg id in GetMessageOrigData, netadr_ip 64-bit OOB, param bounds abort - #390
Open
Nord1cWarr1or wants to merge 1 commit into
Open
fix: original msg id in GetMessageOrigData, netadr_ip 64-bit OOB, param bounds abort#390Nord1cWarr1or wants to merge 1 commit into
Nord1cWarr1or wants to merge 1 commit into
Conversation
…am bounds abort - GetMessageOrigData(MsgMsgId) returned the current message id (getId()); use getOriginalId() like the other Original-branches do (rehlds#387) - set_netadr/get_netadr wrote/read 8 bytes into the 4-byte netadr_s.ip field through a size_t cast - OOB on 64-bit builds; use unsigned int, 32-bit behavior unchanged (rehlds#388) - CHECK_PARAMBOUNDS now aborts the native (return FALSE) after logging instead of continuing with the out-of-bounds argument; all six use-sites reviewed, every caller already has FALSE error paths (rehlds#389)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes three bugs in the message-hook and netaddr natives, one per issue.
1.
GetMessageOrigData(MsgMsgId)returned the current message id — Fixes#387GetMessageOrigDatawithMsgMsgIdreturned the current id (getId()) instead of the original one (getOriginalId()), breaking the native's contract. All other*Orig*branches already used the original getters, andgetOriginalId()has always been part of theIMessageContextinterface (IMessageManager.h).2. Out-of-bounds write/read of
netadr_s.ip[4]viasize_tcast — Fixes#388set_netadr/get_netadrcastnetadr_s.ip(aunsigned char ip[4]field) tosize_t *and wrote/read through it. On LP64size_tis 8 bytes, so 8 bytes were written into a 4-byte field, clobbering the adjacentportand the head ofipx[10]— out-of-bounds access on 64-bit builds.The cast is changed to
unsigned int(4 bytes on both ILP32 and LP64): the OOB is gone, and 32-bit behavior/codegen is unchanged sincesizeof(size_t) == sizeof(unsigned int) == 4there.3.
CHECK_PARAMBOUNDSlogged an error but did not abort the native — Fixes#389The macro reported an AMX runtime error but did not
return, so the native kept executing with the out-of-bounds argument. Addedreturn FALSE;to the macro body. All six use sites (SetMessageData,GetMessageData,GetMessageOrigData,GetMessageArgType,IsMessageDataModified,ResetModifiedMessageData) were audited — each one is acell-returning native with an existingFALSEerror path, so the abort is consistent with the file's error convention.Validation
reapi_amxx_i386.so); the two changed translation units compile with no warnings.-m64) syntax-only compile of the changed translation units: clean.netadr_slayout (common/netadr.h):type(4 bytes) followed byip[4], soipis 4-byte aligned and a 4-byte access is exactly the field width.Fixes#387, Fixes#388, Fixes#389