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

Refactor methods in TextRenderer class #165

Open
SnowyWhite opened this issue Nov 7, 2024 · 2 comments
Open

Refactor methods in TextRenderer class #165

SnowyWhite opened this issue Nov 7, 2024 · 2 comments

Comments

@SnowyWhite
Copy link

What's the problem?

The methods marked in the screenshot below have non-constant bounds:

image

They are used like in this example:

char hostNameBuffer[256]{};
TextRenderer::StripColors(Party::GetHostName().data(), hostNameBuffer, sizeof(hostNameBuffer));
TextRenderer::StripAllTextIcons(hostNameBuffer, hostNameBuffer, sizeof(hostNameBuffer));

The problem here is that we simply trust the method to return only the number of bytes we specify in the last argument.
If it returns more than our buffer can hold, we have a problem.

What might be a solution?

The code must be analysed and an alternative, secure solution implemented.
It is likely that this will also eliminate the method overloads.

@Rackover
Copy link
Contributor

Rackover commented Nov 7, 2024

Am not sure where the problem is - the function is stripping so it's gonna return less by design.
Yes, we trust the function to do so, we have written it to do so! If you look at Strip Colors you will see it cannot build up a bigger string than it was given. Where is the non-secure code that needs fixing? 🤔

@SnowyWhite
Copy link
Author

When I implemented the new exception message, I took the code from Discord.cpp to make the style consistent with the rest of the codebase.
It may not matter in this particular case, but it could introduce a new severe vulnerability in another situation!
Enforcing a secure code style will prevent such potential risks from being overlooked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants