If someone creates the username Guest, all hell breaks loose.

trevor

Hmm, I created names like "banned", "guest", "administrator" because there was no username restrictions in the beginning and banned them from use. I'm glad I did so. Yikes.

Giorgio Chiodi

@trevor makes sense. How can we fix this?

trevor

Okay I changed my name to guest...

Giorgio Chiodi

@guest Oh god an exploit - arrest him!

trevor

Would you change your name as well just to see? But yeah, I'm not sure how to fix this because its a new bug introduced I believe. @baris @julian

EDIT: Change it back because I don't want my name taken lol.

a_5mith

Right, have made some progress with this, @bentael thinks it's down to the SMF importer and the allow guest posting that's been deprecated. So appears not to be a bug with NodeBB per se. But with the importer. As for how I fix this, I'm not sure, but I've closed the github issue as it's not strictly nodebb, but I'll keep this thread going for when one of the devs walks past.

bentael

well, I don't think that the Importer is using the Guest account to preserver the Posts of the deleted or invalid users is an actual bug. It's just a way to keep the topics making sense, so no posts gets lost in the process.

However, respawing the Guest account after deleting it maybe a NodeBB bug or a mis-understanding, I think there is a better way to prevent the Guests from posting.

@julian @baris ?

a_5mith

@bentael @psychobunny

Completing the set. First one in here wins internets. They haven't clocked on they can exploit this yet, but I can't see it being long.

dylenbrivera

The Guest Glitch Exploit. I dig it.

julian

The "guest" itself is not an actual account, per se. We detect whether a guest is posting by looking at the post object and checking for a blank userslug.

e.g.

{
	"pid": "1",
	"uid": "2",
	"tid": "1",
	"content": "<p>This was posted by a real account under the name &quot;Guest&quot;</p>\n",
			-✂- snip snip -✂-
	"user": {
		"username": "Guest",
		"userslug": "guest",
		"reputation": "0",
		"postcount": "1",
		"banned": false,
		"picture": "https://secure.gravatar.com/avatar/c5d5cc05e15e794cdf17459b53e7a793?size=128&default=identicon&rating=pg",
		"signature": "",
		"groups": []
	},
			-✂- snip snip -✂-
}

vs.

{
	"pid": "3",
	"uid": "0",
	"tid": "1",
	"content": "<p>Now I am posting as an actual guest.</p>\n",
			-✂- snip snip -✂-
	"user": {
		"username": "[[global:guest]]",
		"userslug": "",
		"reputation": 0,
		"postcount": "1",
		"banned": false,
		"picture": "https://secure.gravatar.com/avatar/d41d8cd98f00b204e9800998ecf8427e?size=128&default=identicon&rating=pg",
		"signature": "",
		"groups": []
	},
			-✂- snip snip -✂-
}

julian

Now, in hindsight, whoever implemented this checks specifically for the userslug by adding this in the template:

Looking at it now, I believe this was done because our templating engine doesn't parse "0" correctly (interprets it as true), so we can't just check the poster's uid. (Guests have a uid of 0).

As it stands, it seems to be correctly handling the differentiation between a real guest and a user named "Guest". We also don't allow two users to share the same userslug.

We should update templates.js so that an integer uid is returned from getPostData, and interpreted correctly by templates.js...

dylenbrivera

During account creation... if username guest, then have user choose new name. Shouldn't it be this simple? In theory?

julian

Hearing back from @psychobunny now: It seems templates.js interprets "0" as true, and 0 as false (similar to javascript interpretation of those values).

Core should be updated to return integers in the post/topic/category data.
Template should be updated to check the uid instead of a userslug

But this is more just for "better code" purposes... still seeing whether a user named "Guest" can do all sorts of shenanigans...

@dylenbrivera said:

if username guest, then have user choose new name. Shouldn't it be this simple? In theory?

Sure -- but "Guest" is a valid username, technically. No reason why not, from a technological sense, but in a social context, it's not "right", per se.

julian

Note the correct handling in the topic:

User named "Guest"

A real guest

dylenbrivera

@julian Should read "A guest has posted..."

dylenbrivera

That would allow enough room to differentiate.

julian

Have banned a Guest user, user can no longer post, as expected (although there's a "double reply button" glitch, gh#1749, that needs to be taken care of).

User also can't downvote.

a_5mith

Wowzers a lots happened in here since I went off to make Chicken & Chips... So what's the final verdict? Am I safe if I just ban that user?

julian

Issues Identified

Banning a user should log out that user's browser tabs. This used to be the case, but seems to have regressed
issue #1749, as mentioned in the previous post.

At this time, we cannot reproduce the issue of a banned or deleted user being able to downvote another user's posts...

a_5mith

@julian Well, I can't tell if it's just that user, as it doesn't say who downvoted you, there's another "forum" that like to stop by mine every now and then and cause as many issues as they can. The issues of running a forum.