diff --git a/internal/web/service/tgbot/tgbot_callback_auth_test.go b/internal/web/service/tgbot/tgbot_callback_auth_test.go new file mode 100644 index 000000000..6b6f2f019 --- /dev/null +++ b/internal/web/service/tgbot/tgbot_callback_auth_test.go @@ -0,0 +1,46 @@ +package tgbot + +import ( + "testing" + + "github.com/mymmrac/telego" +) + +// A non-admin callback must never reach a privileged handler. The second +// callback switch runs outside the isAdmin guard, so without the default-deny +// check a non-admin who can tap an admin's inline button (e.g. in a group) could +// export the database backup or reset all traffic. Here the privileged handler +// would panic on the nil bot/services of a bare Tgbot; the guard must return +// first, so no panic occurs. +func TestAnswerCallbackDeniesPrivilegedActionToNonAdmin(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("a non-admin callback reached a privileged handler: %v", r) + } + }() + + tg := &Tgbot{} + for _, data := range []string{"get_backup", "reset_all_traffics_c", "add_client", "onlines", "inbounds"} { + q := &telego.CallbackQuery{ + Data: data, + From: telego.User{ID: 999999}, + Message: &telego.Message{Chat: telego.Chat{ID: 1}}, + } + tg.answerCallback(q, false) + } +} + +func TestIsClientSelfCallback(t *testing.T) { + allowed := []string{"client_traffic", "client_sub_links", "client_qr_links", "client_sub_links alice@x"} + for _, d := range allowed { + if !isClientSelfCallback(d) { + t.Errorf("%q should be a per-user client callback", d) + } + } + denied := []string{"get_backup", "reset_all_traffics_c", "add_client", "onlines", "get_banlogs", "get_usage"} + for _, d := range denied { + if isClientSelfCallback(d) { + t.Errorf("%q is an admin-only callback and must not be treated as per-user", d) + } + } +} diff --git a/internal/web/service/tgbot/tgbot_router.go b/internal/web/service/tgbot/tgbot_router.go index 68a89f7a6..74042adf5 100644 --- a/internal/web/service/tgbot/tgbot_router.go +++ b/internal/web/service/tgbot/tgbot_router.go @@ -1128,6 +1128,15 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool } } + // The callbacks below sit outside the isAdmin block above, so a non-admin who + // can see an admin's inline keyboard (for example when the bot runs in a + // group) could otherwise trigger a database backup export, a mass traffic + // reset or client creation. Default-deny: a non-admin may only run the + // per-user client_* callbacks that key off their own Telegram id. + if !isAdmin && !isClientSelfCallback(callbackQuery.Data) { + return + } + switch callbackQuery.Data { case "get_usage": t.sendCallbackAnswerTgBot(callbackQuery.ID, t.I18nBot("tgbot.buttons.serverUsage")) @@ -1526,3 +1535,17 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool func checkAdmin(tgId int64) bool { return slices.Contains(adminIds, tgId) } + +// isClientSelfCallback reports whether a callback is one of the per-user client +// actions that resolve their own data from the caller's Telegram id, and so are +// safe to run for a non-admin. Every other callback is admin-only (default-deny). +func isClientSelfCallback(data string) bool { + switch data { + case "client_traffic", "client_commands", "client_sub_links", + "client_individual_links", "client_qr_links": + return true + } + return strings.HasPrefix(data, "client_sub_links ") || + strings.HasPrefix(data, "client_individual_links ") || + strings.HasPrefix(data, "client_qr_links ") +}