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

TASTEME deobfuscate, comment, and add non-ascii behavior #604

Closed
wants to merge 1 commit into from

Conversation

roleohibachi
Copy link
Contributor

An obfuscated meme function called TASTEME() was introduced ~5mo ago with no documentation. It appears to check for 3 sequential characters in a string; however its behavior is irregular for non-ascii strings, and could be dangerous depending on user implementation. For example, if used for hyperlink validation, it would be vulnerable to IDN homograph attacks.

For example:

input actual result expected result
abcddd True True
aabbcc False False
https://www True True
:// None False
abcçççddd None True
100700070006c0065 None True

This PR de-obfuscates the code, and uses enumerate() and the == operator to make comparisons. This results in more sane/safe behavior.

Since this function was introduced as a meme, it should probably just be removed. But that's like, just my opinion, man.

@paulfitz
Copy link
Member

paulfitz commented Aug 1, 2023

Thanks @roleohibachi ! A lot to unpack here. A small technical issue - I'm seeing strings like Eye of the tiger and Survivor classified as true by TASTEME? https://gristlabs.github.io/grist-static/weresheet.html?p=2#a1.s3.r19.c9

@alexmojaki
Copy link
Contributor

It appears to check for 3 sequential characters in a string

You're close, but I don't think that's correct. I think it's quite fun puzzling out what it's doing, I just enjoyed the experience a second time. 👎 on deobfuscating.

On the other hand, 👍 for overengineering a pointless meme. I'd be in favour of a change that keeps to the style of the original code while handling non-ascii characters better, e.g. removing accents the way that _sanitize_ident does.

@roleohibachi
Copy link
Contributor Author

"Won't fix". I will spend my time on other parts of the project.

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

Successfully merging this pull request may close these issues.

3 participants