Skip to content

fix: 11 bugs including critical data-loss and security issues #3974

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.

Sign up for GitHub

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

Open
mishrarenu066-beep wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: 11 bugs including critical data-loss and security issues #3974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions sub/sub.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"path/filepath"
"strconv"
"strings"
"time"

"github.com/mhsanaei/3x-ui/v2/logger"
"github.com/mhsanaei/3x-ui/v2/util/common"
Expand Down Expand Up @@ -362,16 +363,18 @@ func (s *Server) Start() (err error) {

// Stop gracefully shuts down the subscription server and closes the listener.
func (s *Server) Stop() error {
s.cancel()

var err1 error
var err2 error
if s.httpServer != nil {
err1 = s.httpServer.Shutdown(s.ctx)
// Use a fresh timeout context for graceful shutdown
shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 10*time.Second)
defer shutdownCancel()
err1 = s.httpServer.Shutdown(shutdownCtx)
}
if s.listener != nil {
err2 = s.listener.Close()
}
s.cancel()
return common.Combine(err1, err2)
Comment on lines 374 to 378
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as web server: after httpServer.Shutdown(...) (when serving with Serve(listener)), the listener is usually already closed. The subsequent s.listener.Close() can return net.ErrClosed and bubble up via common.Combine, turning a clean shutdown into an error. Consider skipping the extra close or ignoring net.ErrClosed.

Copilot uses AI. Check for mistakes.
}

Expand Down
30 changes: 14 additions & 16 deletions web/controller/inbound.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (a *InboundController) addInbound(c *gin.Context) {
inbound := &model.Inbound{}
err := c.ShouldBind(inbound)
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundCreateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}
user := session.GetLoginUser(c)
Expand Down Expand Up @@ -136,7 +136,7 @@ func (a *InboundController) addInbound(c *gin.Context) {
func (a *InboundController) delInbound(c *gin.Context) {
id, err := strconv.Atoi(c.Param("id"))
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundDeleteSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}
needRestart, err := a.inboundService.DelInbound(id)
Expand All @@ -158,15 +158,15 @@ func (a *InboundController) delInbound(c *gin.Context) {
func (a *InboundController) updateInbound(c *gin.Context) {
id, err := strconv.Atoi(c.Param("id"))
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}
inbound := &model.Inbound{
Id: id,
}
err = c.ShouldBind(inbound)
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}
inbound, needRestart, err := a.inboundService.UpdateInbound(inbound)
Expand Down Expand Up @@ -234,7 +234,7 @@ func (a *InboundController) clearClientIps(c *gin.Context) {

err := a.inboundService.ClearClientIps(email)
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.updateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.logCleanSuccess"), nil)
Expand All @@ -245,7 +245,7 @@ func (a *InboundController) addInboundClient(c *gin.Context) {
data := &model.Inbound{}
err := c.ShouldBind(data)
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}

Expand All @@ -264,7 +264,7 @@ func (a *InboundController) addInboundClient(c *gin.Context) {
func (a *InboundController) delInboundClient(c *gin.Context) {
id, err := strconv.Atoi(c.Param("id"))
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}
clientId := c.Param("clientId")
Expand All @@ -287,7 +287,7 @@ func (a *InboundController) updateInboundClient(c *gin.Context) {
inbound := &model.Inbound{}
err := c.ShouldBind(inbound)
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}

Expand All @@ -306,7 +306,7 @@ func (a *InboundController) updateInboundClient(c *gin.Context) {
func (a *InboundController) resetClientTraffic(c *gin.Context) {
id, err := strconv.Atoi(c.Param("id"))
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}
email := c.Param("email")
Expand All @@ -328,27 +328,25 @@ func (a *InboundController) resetAllTraffics(c *gin.Context) {
if err != nil {
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
} else {
a.xrayService.SetToNeedRestart()
}
a.xrayService.SetToNeedRestart()
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.resetAllTrafficSuccess"), nil)
}

// resetAllClientTraffics resets traffic counters for all clients in a specific inbound.
func (a *InboundController) resetAllClientTraffics(c *gin.Context) {
id, err := strconv.Atoi(c.Param("id"))
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}

err = a.inboundService.ResetAllClientTraffics(id)
if err != nil {
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
} else {
a.xrayService.SetToNeedRestart()
}
a.xrayService.SetToNeedRestart()
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.resetAllClientTrafficSuccess"), nil)
}

Expand Down Expand Up @@ -386,7 +384,7 @@ func (a *InboundController) importInbound(c *gin.Context) {
func (a *InboundController) delDepletedClients(c *gin.Context) {
id, err := strconv.Atoi(c.Param("id"))
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}
err = a.inboundService.DelDepletedClients(id)
Expand Down Expand Up @@ -421,7 +419,7 @@ func (a *InboundController) updateClientTraffic(c *gin.Context) {
var request TrafficUpdateRequest
err := c.ShouldBindJSON(&request)
if err != nil {
jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err)
jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
return
}

Expand Down
35 changes: 17 additions & 18 deletions web/controller/websocket.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,35 +33,34 @@ var upgrader = ws.Upgrader{
ReadBufferSize: 4096, // Increased from 1024 for better performance
WriteBufferSize: 4096, // Increased from 1024 for better performance
CheckOrigin: func(r *http.Request) bool {
// Check origin for security
origin := r.Header.Get("Origin")
if origin == "" {
// Allow connections without Origin header (same-origin requests)
return true
}
// Get the host from the request
host := r.Host
// Extract scheme and host from origin
originURL := origin
// Simple check: origin should match the request host
// This prevents cross-origin WebSocket hijacking
if strings.HasPrefix(originURL, "http://") || strings.HasPrefix(originURL, "https://") {
// Extract host from origin
originHost := strings.TrimPrefix(strings.TrimPrefix(originURL, "http://"), "https://")

// Extract host from origin
originHost := origin
if strings.HasPrefix(originHost, "http://") || strings.HasPrefix(originHost, "https://") {
originHost = strings.TrimPrefix(strings.TrimPrefix(originHost, "http://"), "https://")
if idx := strings.Index(originHost, "/"); idx != -1 {
originHost = originHost[:idx]
}
if idx := strings.Index(originHost, ":"); idx != -1 {
originHost = originHost[:idx]
}

// Normalize host for comparison (strip ports and IPv6 brackets)
normalizeHost := func(h string) string {
h = strings.TrimPrefix(h, "[")
if idx := strings.LastIndex(h, "]:"); idx != -1 {
h = h[:idx+1]
}
// Compare hosts (without port)
requestHost := host
if idx := strings.Index(requestHost, ":"); idx != -1 {
requestHost = requestHost[:idx]
if idx := strings.LastIndex(h, ":"); idx != -1 && !strings.Contains(h[:idx], "]") {
Comment on lines +53 to +57
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckOrigin host normalization is incorrect for IPv6 addresses (e.g. [2001:db8::1]:443): the LastIndex(":") logic can truncate the IPv6 host, which can cause false matches between different origins and weaken the cross-origin protection. Prefer parsing Origin with url.Parse and comparing u.Hostname() to the request host extracted via net.SplitHostPort/r.Host (and lowercased).

Copilot uses AI. Check for mistakes.
h = h[:idx]
}
return originHost == requestHost || originHost == "" || requestHost == ""
return h
}
return false

return normalizeHost(originHost) == normalizeHost(r.Host)
},
}

Expand Down
2 changes: 1 addition & 1 deletion web/job/check_cpu_usage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (j *CheckCpuJob) Run() {

// get latest status of server
percent, err := cpu.Percent(1*time.Minute, false)
if err == nil && percent[0] > float64(threshold) {
if err == nil && len(percent) > 0 && percent[0] > float64(threshold) {
msg := j.tgbotService.I18nBot("tgbot.messages.cpuThreshold",
"Percent=="+strconv.FormatFloat(percent[0], 'f', 2, 64),
"Threshold=="+strconv.Itoa(threshold))
Expand Down
58 changes: 19 additions & 39 deletions web/service/inbound.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,23 +190,7 @@ func (s *InboundService) checkEmailExistForInbound(inbound *model.Inbound) (stri
if err != nil {
return "", err
}
allEmails, err := s.getAllEmails()
if err != nil {
return "", err
}
var emails []string
for _, client := range clients {
if client.Email != "" {
if s.contains(emails, client.Email) {
return client.Email, nil
}
if s.contains(allEmails, client.Email) {
return client.Email, nil
}
emails = append(emails, client.Email)
}
}
return "", nil
return s.checkEmailsExistForClients(clients)
}

// AddInbound creates a new inbound configuration.
Expand Down Expand Up @@ -339,7 +323,7 @@ func (s *InboundService) DelInbound(id int) (bool, error) {
}
s.xrayApi.Close()
} else {
logger.Debug("No enabled inbound founded to removing by api", tag)
logger.Debug("No enabled inbound found to remove by api for inbound id:", id)
}

// Delete client traffics of inbounds
Expand Down Expand Up @@ -1280,12 +1264,8 @@ func (s *InboundService) disableInvalidClients(tx *gorm.DB) (bool, int64, error)
if strings.Contains(err1.Error(), fmt.Sprintf("User %s not found.", result.Email)) {
logger.Debug("User is already disabled. Nothing to do more...")
} else {
if strings.Contains(err1.Error(), fmt.Sprintf("User %s not found.", result.Email)) {
logger.Debug("User is already disabled. Nothing to do more...")
} else {
logger.Debug("Error in disabling client by api:", err1)
needRestart = true
}
logger.Debug("Error in disabling client by api:", err1)
needRestart = true
}
}
}
Expand Down Expand Up @@ -1458,16 +1438,16 @@ func (s *InboundService) SetClientTelegramUserID(trafficId int, tgId int64) (boo
return false, err
}
clients := settings["clients"].([]any)
var newClients []any
for client_index := range clients {
c := clients[client_index].(map[string]any)
if c["email"] == clientEmail {
c["tgId"] = tgId
c["updated_at"] = time.Now().Unix() * 1000
newClients = append(newClients, any(c))
clients[client_index] = c
break
}
}
settings["clients"] = newClients
settings["clients"] = clients
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Pass only target client into UpdateInboundClient

This change now sends the entire clients array to UpdateInboundClient, but that method still applies only clients[0]/interfaceClients[0] when replacing the matched client entry (see UpdateInboundClient logic around clients[0]). When the target email is not the first client in the inbound, calls like SetClientTelegramUserID will overwrite the target slot with the first client's data, causing duplicate/lost client records and updating the wrong user. The same pattern is repeated in the other updated helpers (ToggleClientEnableByEmail, ResetClientIpLimitByEmail, ResetClientExpiryTimeByEmail, ResetClientTrafficLimitByEmail).

Useful? React with 👍 / 👎.

modifiedSettings, err := json.MarshalIndent(settings, "", " ")
if err != nil {
return false, err
Expand Down Expand Up @@ -1545,16 +1525,16 @@ func (s *InboundService) ToggleClientEnableByEmail(clientEmail string) (bool, bo
return false, false, err
}
clients := settings["clients"].([]any)
var newClients []any
for client_index := range clients {
c := clients[client_index].(map[string]any)
if c["email"] == clientEmail {
c["enable"] = !clientOldEnabled
c["updated_at"] = time.Now().Unix() * 1000
newClients = append(newClients, any(c))
clients[client_index] = c
break
}
}
settings["clients"] = newClients
settings["clients"] = clients
modifiedSettings, err := json.MarshalIndent(settings, "", " ")
if err != nil {
return false, false, err
Expand Down Expand Up @@ -1625,16 +1605,16 @@ func (s *InboundService) ResetClientIpLimitByEmail(clientEmail string, count int
return false, err
}
clients := settings["clients"].([]any)
var newClients []any
for client_index := range clients {
c := clients[client_index].(map[string]any)
if c["email"] == clientEmail {
c["limitIp"] = count
c["updated_at"] = time.Now().Unix() * 1000
newClients = append(newClients, any(c))
clients[client_index] = c
break
}
}
settings["clients"] = newClients
settings["clients"] = clients
modifiedSettings, err := json.MarshalIndent(settings, "", " ")
if err != nil {
return false, err
Expand Down Expand Up @@ -1684,16 +1664,16 @@ func (s *InboundService) ResetClientExpiryTimeByEmail(clientEmail string, expiry
return false, err
}
clients := settings["clients"].([]any)
var newClients []any
for client_index := range clients {
c := clients[client_index].(map[string]any)
if c["email"] == clientEmail {
c["expiryTime"] = expiry_time
c["updated_at"] = time.Now().Unix() * 1000
newClients = append(newClients, any(c))
clients[client_index] = c
break
}
}
settings["clients"] = newClients
settings["clients"] = clients
modifiedSettings, err := json.MarshalIndent(settings, "", " ")
if err != nil {
return false, err
Expand Down Expand Up @@ -1746,16 +1726,16 @@ func (s *InboundService) ResetClientTrafficLimitByEmail(clientEmail string, tota
return false, err
}
clients := settings["clients"].([]any)
var newClients []any
for client_index := range clients {
c := clients[client_index].(map[string]any)
if c["email"] == clientEmail {
c["totalGB"] = totalGB * 1024 * 1024 * 1024
c["updated_at"] = time.Now().Unix() * 1000
newClients = append(newClients, any(c))
clients[client_index] = c
break
}
}
settings["clients"] = newClients
settings["clients"] = clients
modifiedSettings, err := json.MarshalIndent(settings, "", " ")
if err != nil {
return false, err
Expand Down
3 changes: 1 addition & 2 deletions web/service/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,7 @@ func (s *SettingService) ResetSettings() error {
if err != nil {
return err
}
return db.Model(model.User{}).
Where("1 = 1").Error
return db.Where("1 = 1").Delete(model.User{}).Error
}

func (s *SettingService) getSetting(key string) (*model.Setting, error) {
Expand Down
Loading
Loading