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

Fix critical security vulnerability #110

Closed
wants to merge 3 commits into from
Closed

Conversation

antimech
Copy link
Contributor

@antimech antimech commented Jul 11, 2023

Closes botman/studio#99

  • Force Telegram Bot server to send "X-Telegram-Bot-Api-Secret-Token" HTTP header
  • Check the header on update request to the webhook

This is a breaking change all users must run the src/Console/Commands/TelegramRegisterCommand.php console command.

Edits are welcome.

@antimech
Copy link
Contributor Author

@antimech antimech marked this pull request as ready for review October 20, 2023 06:05
@antimech
Copy link
Contributor Author

antimech commented Oct 20, 2023

@mpociot to avoid the breaking change you could make this optional.

@antimech
Copy link
Contributor Author

antimech commented Oct 27, 2023

Tests runs fine on my machine. Seems like to fix it here this needs to be added to the end of the composer.json file:

"config": {
    "allow-plugins": {
        "thecodingmachine/discovery": true
    }
}

and add --ignore-platform-req=composer-plugin-api to tests.yml composer install section.

@filippotoso
Copy link
Contributor

I'm closing this because it's not implemented correctly.

You should not use TELEGRAM_TOKEN as secret_token but a custom secret as per documentation:

https://core.telegram.org/bots/api#setwebhook

Moreover, it must be possible to enable and disable this feature from the configuration file (i.e. secre_token = null, makes it disabled) and make it working also for existing installations (ie. disabled by default).

Feel free to refactor it and send a new PR.

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.

Crititcal Telegram Driver webhook security vulnerability
2 participants