Skip to content

Conversation

Nxllpointer
Copy link
Contributor

Fixes #648

One was able to farm Top-Helper message length points by adding invisible characters to help messages

One was able to farm Top-Helper message length points by adding invisible characters to help messages
@Nxllpointer Nxllpointer requested review from a team as code owners October 21, 2022 14:11
@Zabuzard Zabuzard added bug Something isn't working priority: low labels Oct 21, 2022
@Nxllpointer Nxllpointer requested a review from Zabuzard October 21, 2022 14:35
Copy link
Member

@Zabuzard Zabuzard left a comment

Choose a reason for hiding this comment

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

Can you test it quickly? Just to make sure it really works as expected, thanks.

@Nxllpointer
Copy link
Contributor Author

Can you test it quickly? Just to make sure it really works as expected, thanks.

It works with the zero width joiner. Charachters like äöü for example were also excluded but because the discord is english this should not be an issue

@Nxllpointer Nxllpointer merged commit 5aaba3d into Together-Java:develop Oct 21, 2022
@Zabuzard
Copy link
Member

@Nxllpointer why are umlaute excluded with \P{C}? that category should list all invisible characters. the umlaute are visible. are we really sure we just filtered what we expect and didnt kill everything? please doublecheck that regular characters still count.

@java-coding-prodigy
Copy link
Contributor

@Nxllpointer why are umlaute excluded with \P{C}? that category should list all invisible characters. the umlaute are visible. are we really sure we just filtered what we expect and didnt kill everything? please doublecheck that regular characters still count.

You could do \P{C}|<Whatever the regex for umluate is> in that case

@Zabuzard
Copy link
Member

im okay with what it is. im just confused why its not included. which yields me to thinking that there might be a bug in your code. let me check it on regex101.com quickly

@Nxllpointer
Copy link
Contributor Author

Maybe it's an issue with how java handles Unicode categories

@Zabuzard
Copy link
Member

Maybe it's an issue with how java handles Unicode categories

Are you sure it messes up umlaute? When I test it locally, it works.

test

@Zabuzard
Copy link
Member

Btw, you should have used a Pattern instead of a raw string for the replacement. Would be faster. But welp, too late.

@Nxllpointer
Copy link
Contributor Author

Hm. I used a string because replaceAll does not accept a pattern. Is there an easy readable solution?

@Zabuzard
Copy link
Member

Hm. I used a string because replaceAll does not accept a pattern. Is there an easy readable solution?

replacement via pattern goes through the pattern, not from string. sth like pattern.replace(...).

anyways, lets concentrate on why u think that the code doesnt work for umlaute. please elaborate

@Nxllpointer
Copy link
Contributor Author

Maybe I also can't read a number. If it works for you it should also work for the bot

@Nxllpointer
Copy link
Contributor Author

If you want to I can change it to a pattern later

@Zabuzard
Copy link
Member

ur not getting me. u claimed that it doesnt work for umlaute. why did u say that? did u try it out and it didnt work? do we have a bug in the code?

@Nxllpointer
Copy link
Contributor Author

When I tried it it didn't work. Maybe I just forgot to update the database viewer. You can test if it works.

@Nxllpointer
Copy link
Contributor Author

@Zabuzard oops I just realized that instead of counting the visible chars I counted the invisible ones... I will make a PR with the fix and using Pattern.

@Nxllpointer
Copy link
Contributor Author

Nxllpointer commented Oct 24, 2022

Weird the Java regex seems to ignore the case of the P
Edit: it works again even though i didnt change anything

@Nxllpointer
Copy link
Contributor Author

PR should fix everything now. Sorry for this stupid mistake.

@Nxllpointer Nxllpointer deleted the fix/top-helper-exploit branch October 24, 2022 21:20
@Zabuzard Zabuzard mentioned this pull request Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Top-Helper message length exploit

4 participants