Skip to content

Commit b9526b3

Browse files
committed
fix(users): harden password hash handling in ModifyPasswd
Validate password crypt hashes and username before invoking chpasswd to reject invalid characters and malformed input per crypt(5) and useradd's username validation rules. Also improve process isolation and sensitive data handling by clearing the child environment, switching to an explicit stdin pipe flow. Signed-off-by: Yuming He <heyuming@deepin.org>
1 parent 2e39d84 commit b9526b3

2 files changed

Lines changed: 164 additions & 13 deletions

File tree

accounts1/users/prop.go

Lines changed: 128 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@ import (
2020
libdate "github.com/rickb777/date"
2121
)
2222

23-
var (
24-
errInvalidParam = fmt.Errorf("Invalid or empty parameter")
25-
)
23+
var errInvalidParam = fmt.Errorf("Invalid or empty parameter")
2624

2725
var (
2826
groupFileTimestamp int64 = 0
@@ -157,25 +155,142 @@ func ModifyShell(shell, username string) error {
157155
return doAction(userCmdModify, []string{"-s", shell, username})
158156
}
159157

158+
// isValidCryptHash validates the format of a crypt password hash string.
159+
// It enforces printable ASCII boundaries and blocks high-risk delimiters.
160+
// from: https://manpages.debian.org/unstable/libcrypt-dev/crypt.5.en.html
161+
func isValidCryptHash(hash string) error {
162+
if hash == "" {
163+
return errors.New("password hash is empty")
164+
}
165+
166+
for i := 0; i < len(hash); i++ {
167+
b := hash[i]
168+
169+
// This keeps the error generic and avoids leaking hash structure details.
170+
if b < 32 || b > 126 {
171+
return errors.New("password hash contains non-printable ASCII characters")
172+
}
173+
174+
switch b {
175+
case ' ', ':', ';', '*', '!', '\\':
176+
return errors.New("password hash contains forbidden characters")
177+
}
178+
}
179+
180+
return nil
181+
}
182+
183+
// isValidUsername validates the input username.
184+
// It follows useradd's strict rules instead of adduser's NAME_REGEX
185+
// to prevent control flow injection (e.g., line/field truncation)
186+
// when feeding "username:password" into chpasswd via stdin.
187+
// from: https://github.com/shadow-maint/shadow/blob/710c4d4f88fa32dfc4c4d1f714e935d8bff6ae00/lib/chkname.c#L103
188+
func isValidUsername(name string) error {
189+
if name == "" || name == "." || name == ".." {
190+
return errors.New("username can't be '.' or '..' or empty")
191+
}
192+
193+
if len(name) > LoginNameMaxSize() {
194+
return errors.New("username too long")
195+
}
196+
197+
if strings.Trim(name, "-") == "" {
198+
return errors.New("username cannot consist entirely of hyphens")
199+
}
200+
201+
// below check follows BRE: [a-zA-Z0-9_.][a-zA-Z0-9_.-]*$\?
202+
first := name[0]
203+
isFirstValid := (first >= 'a' && first <= 'z') ||
204+
(first >= 'A' && first <= 'Z') ||
205+
(first >= '0' && first <= '9') ||
206+
first == '_' ||
207+
first == '.'
208+
if !isFirstValid {
209+
return errors.New("first character must be alphanumeric, underscore, or dot")
210+
}
211+
212+
isAllDigit := (first >= '0' && first <= '9')
213+
for i := 1; i < len(name); i++ {
214+
ch := name[i]
215+
216+
if ch < '0' || ch > '9' {
217+
isAllDigit = false
218+
}
219+
220+
isValidChar := (ch >= 'a' && ch <= 'z') ||
221+
(ch >= 'A' && ch <= 'Z') ||
222+
(ch >= '0' && ch <= '9') ||
223+
ch == '_' ||
224+
ch == '.' ||
225+
ch == '-'
226+
227+
if isValidChar {
228+
continue
229+
}
230+
231+
if ch == '$' && i == len(name)-1 {
232+
continue
233+
}
234+
235+
return errors.New("username contains invalid characters or '$' is not at the end")
236+
}
237+
238+
if isAllDigit {
239+
return errors.New("username cannot consist entirely of digits")
240+
}
241+
242+
return nil
243+
}
244+
160245
func ModifyPasswd(words, username string) error {
161-
if len(words) == 0 {
162-
return errInvalidParam
246+
if words == "" || username == "" {
247+
return errors.New("password hash or username is empty")
163248
}
164-
// 防止命令注入
165-
if strings.ContainsAny(words, "\n\r") || strings.ContainsAny(username, "\n\r:") {
166-
return errInvalidParam
249+
250+
if err := isValidUsername(username); err != nil {
251+
return fmt.Errorf("username is invalid: %w", err)
252+
}
253+
254+
if err := isValidCryptHash(words); err != nil {
255+
return fmt.Errorf("invalid password hash: %w", err)
167256
}
168257

169258
cmd := exec.Command(pwdCmdModify, "-e")
170-
input := fmt.Sprintf("%s:%s\n", username, words)
171-
cmd.Stdin = bytes.NewBufferString(input)
259+
// clear environments for security, if it works unexpectedly then add env which chpasswd needs
260+
cmd.Env = []string{}
261+
262+
stdin, err := cmd.StdinPipe()
263+
if err != nil {
264+
return fmt.Errorf("failed to create stdin pipe: %w", err)
265+
}
172266

173267
var stderr bytes.Buffer
174268
cmd.Stderr = &stderr
175269

176-
err := cmd.Run()
177-
if err != nil {
178-
return fmt.Errorf("failed to modify password: %v, %s", err, stderr.String())
270+
if err := cmd.Start(); err != nil {
271+
return fmt.Errorf("failed to start command: %w", err)
272+
}
273+
274+
// Write the password hash to stdin
275+
// no need to erase this data, because this hash already exist in go string
276+
buf := bytes.NewBuffer(make([]byte, 0, len(username)+len(words)+2))
277+
buf.WriteString(username)
278+
buf.WriteString(":")
279+
buf.WriteString(words)
280+
buf.WriteString("\n")
281+
282+
input := buf.Bytes()
283+
_, writeErr := stdin.Write(input)
284+
stdin.Close()
285+
286+
if writeErr != nil {
287+
_ = cmd.Process.Kill()
288+
_ = cmd.Wait()
289+
return fmt.Errorf("failed to write to stdin: %w", writeErr)
290+
}
291+
292+
if err := cmd.Wait(); err != nil {
293+
return fmt.Errorf("failed to update system password configuration %s", stderr.String())
179294
}
180295

181296
return nil

accounts1/users/user.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// SPDX-FileCopyrightText: 2026 UnionTech Software Technology Co., Ltd.
2+
//
3+
// SPDX-License-Identifier: GPL-3.0-or-later
4+
5+
package users
6+
7+
/*
8+
#include <unistd.h>
9+
#include <limits.h>
10+
11+
#ifndef LOGIN_NAME_MAX
12+
#define LOGIN_NAME_MAX 256
13+
#endif
14+
15+
long get_login_name_max() {
16+
long conf = -1;
17+
conf = sysconf(_SC_LOGIN_NAME_MAX);
18+
19+
if (conf == -1) {
20+
conf = LOGIN_NAME_MAX;
21+
}
22+
23+
return conf;
24+
}
25+
*/
26+
import "C"
27+
28+
var loginNameMaxSize int
29+
30+
func init() {
31+
loginNameMaxSize = int(C.get_login_name_max())
32+
}
33+
34+
func LoginNameMaxSize() int {
35+
return loginNameMaxSize
36+
}

0 commit comments

Comments
 (0)