Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix GetPacketID assertion failure in src/Hooks.cpp #65

Merged

Conversation

najuaircrack
Copy link
Contributor

This pull request fixes an assertion failure in GetPacketID located in src/Hooks.cpp.

The assertion p->length > sizeof(unsigned char) + sizeof(unsigned long)` was failing due to incorrect handling of packet lengths. This fix ensures the packet length is properly validated before accessing the data.

samp03svr: /home/michael/repos/SKY/src/Hooks.cpp:148: BYTE GetPacketID(Packet*): Assertion `p->length > sizeof(unsigned char) + sizeof(unsigned long)' failed.
zsh: IOT instruction ./samp03svr

I know that this is caused by a crasher.

@najuaircrack najuaircrack deleted the fix-getpacketid-assertion branch January 18, 2025 09:41
@najuaircrack najuaircrack restored the fix-getpacketid-assertion branch January 18, 2025 09:55
@najuaircrack najuaircrack reopened this Jan 18, 2025
@najuaircrack
Copy link
Contributor Author

I think there is no need to terminate the program if the packet length is short

@najuaircrack
Copy link
Contributor Author

somebody is now exploiting this to crash servers.

src/Hooks.cpp Outdated Show resolved Hide resolved
@ADRFranklin
Copy link
Collaborator

You are right, we should not be asserting this and killing the process because of a bad packet. It should be ignored. Not sure why this managed to go unnoticed for so long.

Thanks for the PR

@najuaircrack
Copy link
Contributor Author

I just put the log to confirm and identify . i removed it !

@ADRFranklin ADRFranklin merged commit da0c976 into oscar-broman:master Jan 19, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants