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

Security vulnerability: user can become a channel operator by accident #2201

Open
4 of 10 tasks
cyrmax opened this issue Mar 11, 2024 · 8 comments
Open
4 of 10 tasks

Security vulnerability: user can become a channel operator by accident #2201

cyrmax opened this issue Mar 11, 2024 · 8 comments

Comments

@cyrmax
Copy link

cyrmax commented Mar 11, 2024

Description

This problem occurs because the information about channel operators is stored in the form where only the channel ID is recorded.
So if the administrator deletes some channel and creates a new one with the same ID, all users who had some extra rights to the old channel immediately get the same rights in the new one.

Not checked yet, but I think that with other extra rights and per user settings this situation will be the same.

Application

  • qtTeamTalk
  • TeamTalkAndroid
  • iTeamTalk
  • TeamTalkClassic
  • TeamTalkServer

Platform

  • Windows
  • macOS
  • Android
  • iOS
  • Linux

Expected behavior

I suggest that when some channel is being deleted while server is running and when server starts / stops and checks the configuration all records that belong to channels that don't exist anymore should be also deleted from the configuration.

Actual behavior

When you delete a channel and create a new one and it gets the same ID as the old one, users who were old channel operators become the new channel operators too.

Steps to reproduce problem

  1. Create a permanent channel on your server;
  2. Set some user as the channel operator;
  3. Open Teamtalk config xml file and write down an ID of the newly created channel;
  4. Delete the channel;
  5. Create a new permanent channel;
  6. Notice that its ID is the same as the deleted one;
  7. Ask the user who was the previous channel operator to re-login;
  8. Find that now he/she is a new channel operator too.
@cyrmax cyrmax added the bug label Mar 11, 2024
@beqabeqa473
Copy link
Contributor

Not checked yet, but I think that with other extra rights and per user settings this situation will be the same.

Could you elaborate about this?

@cyrmax
Copy link
Author

cyrmax commented Mar 11, 2024

@beqabeqa473 Excuse my hasty unchecked conclusions.
Now I have checked and everything besides this operator thing works without any problems.

@peter1384
Copy link
Contributor

This only happens with channel operators and it doesn't happen with other user rights because only channel operator user's ids cunflicts with each other.

@beqabeqa473
Copy link
Contributor

@bear101 could you please give me a hint where to look? i'd like to fix that if you don't have enough time. I tried looking, but quick look didn't show up anything.

@bear101
Copy link
Contributor

bear101 commented Mar 16, 2024

This is where it stores channel-IDs: https://github.com/BearWare/TeamTalk5/blob/master/Library/TeamTalkLib/bin/ttsrv/ServerXML.cpp#L1322

It should store channel-path instead.

If you change it then make sure to write support for the modification in ServerXML::UpdateFile: https://github.com/BearWare/TeamTalk5/blob/master/Library/TeamTalkLib/bin/ttsrv/ServerXML.cpp#L1322

@beqabeqa473
Copy link
Contributor

Hi.

I don't think this is good idea.

If we are generating a channel, with random id by request, and if such id will be repeated in the future, old chan OP will become also operator of this new channel.

I think, to fix it correctly, on channel remove, all occurences of that chanid should be removed from all available users.

@peter1384
Copy link
Contributor

I agree with @beqabeqa473 here, because when a channel is being removed, all related informations and data should be removed from that channel. I've noticed that when a user is a channel operator and that channel gets removed, the user is stil operator of that channel when looking at their user account, Which is bad.

@amirmahdifard
Copy link

I think that what @bear101 said should be done, because if a user is the operator of the audio box channel, And the audio box channel got removed and another channel with another name has been created, The user should not become operator, but if a channel made again with the name audio box, the user should become operator again in the channel audio box until some admin manually remove the operator of that channel for the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants