Skip to content

fix: disable SetPassword DBus method and prevent chpasswd injection #1119

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

Merged
caixr23 merged 1 commit into from
May 22, 2026
Merged

fix: disable SetPassword DBus method and prevent chpasswd injection #1119

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
50 changes: 1 addition & 49 deletions accounts1/user_ifc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,55 +116,7 @@ func (u *User) SetShell(sender dbus.Sender, shell string) *dbus.Error {
}

func (u *User) SetPassword(sender dbus.Sender, password string) *dbus.Error {
logger.Debug("[SetPassword] start ...")

// set password from UnionID
if password == "" {
return nil
}

err := u.checkAuth(sender, false, "")
if err != nil {
logger.Debug("[SetPassword] access denied:", err)
return dbusutil.ToError(err)
}

var count = 10
for {
_, err := users.GetShadowInfo(u.UserName)

if err == nil {
break
}
count--
if count == 0 {
return dbusutil.ToError(err)
}
time.Sleep(time.Second)
}

if err := users.ModifyPasswd(password, u.UserName); err != nil {
logger.Warning("DoAction: modify password failed:", err)
return dbusutil.ToError(err)
}

err = removeLoginKeyring(u)
if err != nil {
logger.Warningf("DoAction: remove login keyring failed: %v", err)
}

u.PropsMu.Lock()
defer u.PropsMu.Unlock()

if u.Locked {
if err := users.LockedUser(false, u.UserName); err != nil {
logger.Warning("DoAction: unlock user failed:", err)
return dbusutil.ToError(err)
}
u.Locked = false
_ = u.emitPropChangedLocked(false)
}
return nil
return dbusutil.ToError(fmt.Errorf("SetPassword is deprecated and no longer supported"))
}

func (u *User) SetMaxPasswordAge(sender dbus.Sender, nDays int32) *dbus.Error {
Expand Down
4 changes: 4 additions & 0 deletions accounts1/users/prop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ func ModifyPasswd(words, username string) error {
if len(words) == 0 {
return errInvalidParam
}
// 防止命令注入
if strings.ContainsAny(words, "\n\r") || strings.ContainsAny(username, "\n\r:") {
return errInvalidParam
}
Comment on lines +165 to +167
Copy link
Copy Markdown
Member

@UTsweetyfish UTsweetyfish May 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Hashed passphrases are always entirely printable ASCII, and do not contain any whitespace or the characters ‘:’, ‘;’, ‘*’, ‘!’, or ‘\’. (These characters are used as delimiters and special markers in the passwd(5) and shadow(5) files.)

这里说的就是哈希后的密码不应包含冒号等

https://manpages.debian.org/unstable/libcrypt-dev/crypt.5.en.html

Copy link
Copy Markdown
Member

@UTsweetyfish UTsweetyfish May 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

使用更严格的白名单校验：对于 username，应该只允许字母、数字、下划线和连字符。对于 words（密码），虽然允许的字符较多，但仍建议过滤掉所有控制字符（ASCII < 32）。

我也建议用白名单校验，
先只允许 printable ASCII，再禁掉 :;*!\

Copy link
Copy Markdown
Contributor Author

@caixr23 caixr23 May 22, 2026

Choose a reason for hiding this comment

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

words是加密后的密码，\n已经能防注入了，先保证不影响原有功能


cmd := exec.Command(pwdCmdModify, "-e")
input := fmt.Sprintf("%s:%s\n", username, words)
Expand Down
Loading