DarkflameUniverse / DarkflameServer

The main repository for the Darkflame Universe Server Emulator project.
https://www.darkflameuniverse.org/
GNU Affero General Public License v3.0
639 stars 172 forks source link

fix: ignore whitespace in TryParse #1536

Closed EmosewaMC closed 5 months ago

EmosewaMC commented 5 months ago

tested that Not the Boss of Me progresses normally (its cdclient entry is 7805,11225,11226, 11217, 11220, 11214, 11982, 11987, 11989, 12005, 11999, 12467, 12468, 12469, 12591, 12600, 12609, 14381,16047,16048,16049,16050

jadebenn commented 5 months ago

Is there any possibility here that this could overrun the string_view .size()? I think you'd need a pretty choice confluence of circumstances (string made of whitespace, no null termination, following byte matching a whitespace character), but I think it's possible to invoke UB. Would it make sense to add a length check to the loop?

EmosewaMC commented 5 months ago

Is there any possibility here that this could overrun the string_view .size()? I think you'd need a pretty choice confluence of circumstances (string made of whitespace, no null termination, following byte matching a whitespace character), but I think it's possible to invoke UB. Would it make sense to add a length check to the loop?

.empty already does this check and .front will throw before allowing ub.

EmosewaMC commented 5 months ago

I can see how this may overrun (it wont because null termination would result in false and just return). Feel free to make a patch with a working check.

EmosewaMC commented 5 months ago

Is there any possibility here that this could overrun the string_view .size()? I think you'd need a pretty choice confluence of circumstances (string made of whitespace, no null termination, following byte matching a whitespace character), but I think it's possible to invoke UB. Would it make sense to add a length check to the loop?

.empty already does this check and .front will throw before allowing ub.

correction, front does not throw, which is very different from the behavior I had thought front did. not sure where I got that information from.