Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add bounds check for userid reset on disconnect #1108

Merged
merged 1 commit into from
Oct 31, 2019
Merged

Conversation

Headline
Copy link
Member

Fixes #1107

Left out the check for player connection since it may be impossible for IVEngineServer::GetPlayerUserId to return -1 at the point in time we ask for it.

@Headline Headline added the Bug general bugs; can be anything label Oct 30, 2019
@asherkin
Copy link
Member

I wonder if we should scan the array instead in the failure case - we know it is in there somewhere and we don't really want to leave bad data behind, or is that fine? (I haven't fully parsed all the context of this yet, it's pretty late.)

@Headline
Copy link
Member Author

Headline commented Oct 30, 2019

@asherkin we’d be doing a huge iteration which would raise some performance implications. Approximately 65k

If userid is always increasing, I presume it’s fine to just leave bad data in, but that allows for the possibility of getclientuserid to return an invalid client index that’s not -1

honestly probably needs a hashset

Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops - yeah, I initially parsed this as being the other way.

It looks like GetClientOfUserId already expects this cache to be garbage sometimes, so :shipit:. (Although it might be worth updating / removing the comment about older engines there.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug general bugs; can be anything
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PlayerManager functions cause heap corruption when engine->GetPlayerUserId() returns -1
3 participants