Possible attack vector -- superuser username spoofing?

[marinegor]

I wonder what would happen in the following scenario:

• I run the bot with --super=someuser
• after some time, someuser changes nickname to anotheruser
• a malicious user names themselves someuser and sends /spam command

Would the bot recognise it as a valid command or not?

[umputun]

This is possible, but I don't think it's unexpected behavior. Having @username configured anywhere would be a concern if the username was changed later on. Perhaps we could make it use ID instead of or in addition to the name, but obtaining the ID in Telegram is not easy, and I don't want to force users who are okay with keeping their names into this dance.

Personally, I don't see this as a security issue. However, if someone adds an optional check for ID to SuperUsers.IsSuper, we could set IDs optionally instead of names.

[johnd0e]

but obtaining the ID in Telegram

What do you mean?
Isn't that the similar to getting admin group id?

And instead of using 3rd-party tools, tg-spam could process both cases with ease, just writing that info into log or (better!) to chat.

[umputun]

This is a configuration step that should be completed before tg-spam is started. Sure, one can get this ID from the log already, but the flow seems to be awkward - run tg-spam with the name only, write down the IDs of admins, update the config, and restart tg-spam.

As I said, I don't see this as a real problem I need to spend time on, but if someone wants to implement it, I'm fine with that.

[johnd0e]

This is a configuration step that should be completed before starting tg-spam.

Yes, but this is exactly the same for the admin group ID. In both cases, it is possible to use third-party bots to obtain the ID.

But I personally tend to avoid that: currently, my method involves checking the logs or directly querying the bot API endpoint myself. However, it would be better to implement user-friendly native options in tg-spam instead.

As I mentioned, I don't see this as a significant problem that requires my attention.

Indeed, the issue raised in the title is not very serious. I am more of an advocate for overall user experience. The ability to use not only a @nickname (which is not mandatory in Telegram) but also a numerical user ID would be a valuable improvement.

[umputun]

the change is on master #156

pls, give it a try

[johnd0e]

Now all seems ok except log entries:

2024/10/21 00:56:15.860 [DEBUG] {events/listener.go:348 events.(*TelegramListener).isAdminChat} message in admin chat -1002332655974, from
2024/10/21 00:56:15.860 [DEBUG] {events/admin.go:64 events.(*admin).MsgHandler} message from admin chat: msg id: 27, update id: 223500513, from: , sender: ""

It would be nice to output id and first_name/last_name instead of empty username.

[umputun]

I have added the ID in several places where the user name was reported before. I'm not sure I covered them all, but most of the places should be fine now. I have not added first or last names, however, just the extra (ID).

[johnd0e]

See the last line: from: , sender: "" (0)

[DEBUG] {events/listener.go:348 events.(*TelegramListener).isAdminChat} message in admin chat -1002332655974, from  (897129611)
2024/10/21 14:33:58.669 [DEBUG] {events/admin.go:64 events.(*admin).MsgHandler} message from admin chat: msg id: 31, update id: 223500517, from: , sender: "" (0)

Also I wonder if --admin option is adapted as well.

[umputun]

The tgbot API is odd in this place. There is a high-level field for the forwarder sender name; however, not the ID. The ID is available under an optional ForwardFrom structure, which in some cases can be nil. I'm not sure why this distinction exists, probably related to the privacy settings. Anyway, added a code that will try to extract the id from ForwardFrom.

Also I wonder if --admin option is adapted as well.

yes, see

tg-spam/app/events/listener.go

Line 411 in a2e5a78

func (l *TelegramListener) updateSupers() error {

[johnd0e]

The ID is available under an optional ForwardFrom structure,

But why. The mentioned 2 lines from the log are result of me just posting "hi" in admin chat, so there is no forwarding there.

And when I see the tg API response (eg in browser) - there are all required fields.

Anyway, added a code that will try to extract the id from ForwardFrom.

Unfortunately no difference

[umputun]

Hmm, I'm not sure why this is even a concern. TG-spam is not trying to be a universal logging tool recording who and how interacted with the admin chat. The goal of the logging is to capture details on the forwarded message, which is the only thing related to TG-spam functionality. The debug message on any message in the admin chat in this case doesn't add much to it if it wasn't the actual forward, but this is debug level, so some noise is to be expected.