I've just discovered that there are two member accounts with the same display name on this forum.
I only allow admins and buddies to send me PMs and it seems there are two forum members with the name RockLee because when I tried to add the member RockLee using the 'Add to Buddy List' search function it added the member with the name rocklee to my buddy list even though only the name RockLee was shown in the suggestion list and that was what I selected.
The only way I could add the member RockLee to my buddy list was via their profile.
It seems that name checking wasn't case sensitive at some point in time (hopefully it is now) but this is an issue when you either can't find the member you're searching for (ie, RockLee is not listed in the Memberlist when I search for the name RockLee - only the member rocklee is listed) or the wrong member is selected when you choose a name from a suggestion list (ie, as happened when I tried to add RockLee as a buddy).
Turns out the member RockLee (https://www.simplemachines.org/community/index.php?action=profile;u=322597) has a hidden character between 'Rock' and 'Lee' in their display name so if I search for 'rock' in the memberlist his account is listed (but not if I search for 'rocklee').
Interesting, but similar things have popped up before as well. It's an old issue with how SMF checks (checked? Not sure if this has been addressed) for reserved usernames, that can't really be automatically fixed for existing accounts retroactively.
Quote from: Aleksi "Lex" Kilpinen on September 13, 2021, 11:45:31 AMInteresting, but similar things have popped up before as well. It's an old issue with how SMF checks (checked? Not sure if this has been addressed) for reserved usernames, that can't really be automatically fixed for existing accounts retroactively.
I've changed this to 'not solved' because I believe that autosuggest adding a different member to the one that was selected from the list is an issue (the memberlist search is less of an issue an can be worked around by changing the search criteria).
Should we move it to the bugs board? :P
Quote from: Diego Andrés on September 14, 2021, 07:53:58 PMShould we move it to the bugs board? :P
i am not sure it is a big enough bug to be worth the devs time to fix it. this has existed forever and very few have exploited it.
Quote from: Illori on September 15, 2021, 04:55:39 AMQuote from: Diego Andrés on September 14, 2021, 07:53:58 PMShould we move it to the bugs board? :P
i am not sure it is a big enough bug to be worth the devs time to fix it. this has existed forever and very few have exploited it.
Fair enough - just wanted to make sure someone knew about it ...
It's certainly been an issue in the past where names were spoofed with invisible characters at the start or end, but none of us figured that fixing names in the middle was a concern at the time.
Perhaps it should have been... what character is it? Perhaps this is not a big fix.
It's Rock␏Lee - I really don't know how to identify that character any better. It's actually only in the display name, so this wasn't even the exact issue I thought it was.
EDIT: Hmm. Seems that character does not like copy/paste - It appears different in all three places, where I copied it from, when I pasted it, and after it has been posted :D
It appears to be 0x0F, or u+000F if you like, which is the control code for Shift-In, which generally speaking would not be a printable character (so I'm surprised the SI showed up in Lex's post).
You know, I think this is a bug.
U+000F is stripped in non UTF8 mode but apparently not in UTF8 mode because I wrote the regex wrong in 2013 in 2.0.6!
The regex in https://github.com/SimpleMachines/SMF2.1/commit/adce8eb20f84b392530abd3e33e5de832e3cc7e1 for example has 0x0e through 0x19 as excluded which should exclude 0x0f but I'm wondering if it should actually exclude 0x0e through 0x1f instead (and do so in the UTF8 branch which it currently does not)
Moved to bugs, warrants a deeper look at least I think.
Yep, it's definitely still possible to put 0x0F not only into the display name but also the real name upon registration as well. I do think we need to fix this, as putting non graphic characters in the display name at least can be used for ill intent to impersonate others, if not just confusing for others.
As far as I know that's all the control characters, 0x00 to 0x1F and 0x7F to 0x9F that need to be excluded.
Of course, fixing this won't help against those that have already done it
Excluding 0x7f to 0x9f is complicated in UTF-8 land.
But in principle I agree with the rest of it. I don't know why I didn't just do that originally though.
Not sure why you'd say 0x7f to 0x9f is complicated, the basic testing I did to detect those characters worked fine. You'll have to enlighten me.
Anyway, a bug report has been filed over at GitHub so we can fix it.
Because characters and bytes are not the same thing, and characters up past 0x7f are specially encoded and extra care needs to be taken.
E.g. 0xA0 and U+00A0 are absolutely not the same thing. (Incidentally U+00A0 *is* on the excluded list.)
This is a particular manifestation of some broader issues with SMF's string handling. I'm currently working on a series of fixes to deal with Unicode normalization, improved input sanitization and validation, etc.
Regarding this particular issue (string spoofing using invisible characters), my upcoming changes should prevent new input in user profile fields from including invisible characters. However, retroactively replacing invisible characters in preexisting data could easily cause other problems (and it would be a painful procedure for a large forum like this). But applying string substitution on the fly to retrieved data before displaying it, such as replacing invisible characters with the Unicode Replacement Character (U+FFFD), should help defeat spoofing attempts like this.
Quote from: Arantor on September 16, 2021, 11:07:10 AMBecause characters and bytes are not the same thing, and characters up past 0x7f are specially encoded and extra care needs to be taken.
E.g. 0xA0 and U+00A0 are absolutely not the same thing. (Incidentally U+00A0 *is* on the excluded list.)
Ah yes, I get you, my bad for the confusion, I was using 0x notation even though I'm meaning u+ notation (because it's still a hex number). So you are correct, u+009F would be 0xC2 0x9F and checking that way I imagine would be a nightmare but I do believe we can simply check against the u+ notation like you had done already, at least it's working for me doing it that way for those characters.
So this is me pretending to be vbgamer45 by using the same invisible character in my display name that the member RockLee used in their display name ...
You cannot view this attachment.
And I can even pretend to be Arantor too!!
You cannot view this attachment.
This is another reason why you should never let your users to change their displayname. 8)
Quote from: asmith on September 17, 2021, 01:40:02 AMThis is another reason why you should never let your users to change their displayname. 8)
Except that, and as below, it's also currently possible for new members to include hidden characters in their real name when they register ...
Quote from: Chief of Nothing on September 16, 2021, 03:19:40 AMYep, it's definitely still possible to put 0x0F not only into the display name but also the real name upon registration as well. I do think we need to fix this, as putting non graphic characters in the display name at least can be used for ill intent to impersonate others, if not just confusing for others.
Quote from: asmith on September 17, 2021, 01:40:02 AMThis is another reason why you should never let your users to change their displayname. 8)
Or you could allow it and let people have some additional security for free (Arantor is not my username)
Quote from: Arantor on September 17, 2021, 03:55:17 AMOr you could allow it and let people have some additional security for free (Arantor is not my username)
It actually depends on the forum content and who it is aimed at. For example gamers tend to change their nicknames a lot and allowing them to change their display name often creates confusion for other members.