Skip to content

Commit 58dd5fd

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, and zeroing the temporary password buffer after use. Additionally, avoid exposing detailed backend errors to callers to reduce information disclosure risks. Signed-off-by: ComixHe <heyuming@deepin.org>
1 parent 6741354 commit 58dd5fd

2 files changed

Lines changed: 164 additions & 13 deletions

File tree

accounts1/users/prop.go

Lines changed: 138 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,152 @@ 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+
// IsValidName 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 len(name) > LoginNameMaxSize() {
190+
return errors.New("user name too long")
191+
}
192+
193+
if name == "" || name == "." || name == ".." {
194+
return errors.New("username can't be '.' or '..' or empty")
195+
}
196+
197+
if strings.Trim(name, "-") == "" {
198+
return errors.New("username cannot consist entirely of hyphens")
199+
}
200+
201+
if strings.ContainsAny(name, " \"#',/:;") {
202+
return errors.New("username contains forbidden characters (space, \", #, ', ,, /, :, ;)")
203+
}
204+
205+
isAllDigit := true
206+
for i := 0; i < len(name); i++ {
207+
ch := name[i]
208+
if ch <= 0x1F || ch == 0x7F {
209+
return errors.New("username cannot contain control characters")
210+
}
211+
212+
if ch < '0' || ch > '9' {
213+
isAllDigit = false
214+
}
215+
}
216+
217+
if isAllDigit {
218+
return errors.New("username cannot consist entirely of digits")
219+
}
220+
221+
// below check follows BRE: [a-zA-Z0-9_.][a-zA-Z0-9_.-]*$\?
222+
first := name[0]
223+
isFirstValid := (first >= 'a' && first <= 'z') ||
224+
(first >= 'A' && first <= 'Z') ||
225+
(first >= '0' && first <= '9') ||
226+
first == '_' ||
227+
first == '.'
228+
if !isFirstValid {
229+
return errors.New("first character must be alphanumeric, underscore, or dot")
230+
}
231+
232+
for i := 1; i < len(name); i++ {
233+
ch := name[i]
234+
235+
isValidChar := (ch >= 'a' && ch <= 'z') ||
236+
(ch >= 'A' && ch <= 'Z') ||
237+
(ch >= '0' && ch <= '9') ||
238+
ch == '_' ||
239+
ch == '.' ||
240+
ch == '-'
241+
242+
if isValidChar {
243+
continue
244+
}
245+
246+
if ch == '$' && i == len(name)-1 {
247+
continue
248+
}
249+
250+
return errors.New("username contains invalid characters or '$' is not at the end")
251+
}
252+
253+
return nil
254+
}
255+
160256
func ModifyPasswd(words, username string) error {
161-
if len(words) == 0 {
162-
return errInvalidParam
257+
if words == "" || username == "" {
258+
return errors.New("password hash or username is empty")
163259
}
164-
// 防止命令注入
165-
if strings.ContainsAny(words, "\n\r") || strings.ContainsAny(username, "\n\r:") {
166-
return errInvalidParam
260+
261+
if err := isValidUsername(username); err != nil {
262+
return fmt.Errorf("username is invalid: %w", err)
263+
}
264+
265+
if err := IsValidCryptHash(words); err != nil {
266+
return fmt.Errorf("invalid password hash: %w", err)
167267
}
168268

169269
cmd := exec.Command(pwdCmdModify, "-e")
170-
input := fmt.Sprintf("%s:%s\n", username, words)
171-
cmd.Stdin = bytes.NewBufferString(input)
270+
cmd.Env = []string{}
271+
272+
stdin, err := cmd.StdinPipe()
273+
if err != nil {
274+
return fmt.Errorf("failed to create stdin pipe: %w", err)
275+
}
172276

173277
var stderr bytes.Buffer
174278
cmd.Stderr = &stderr
175279

176-
err := cmd.Run()
177-
if err != nil {
178-
return fmt.Errorf("failed to modify password: %v, %s", err, stderr.String())
280+
if err := cmd.Start(); err != nil {
281+
return fmt.Errorf("failed to start command: %w", err)
282+
}
283+
284+
// Write the password hash to stdin
285+
input := []byte(username + ":" + words + "\n")
286+
_, writeErr := stdin.Write(input)
287+
for i := range input {
288+
input[i] = 0
289+
}
290+
291+
if len(input) > 0 && input[0] != 0 {
292+
_ = input[0] // forbid DCE and ensure input is zeroed out
293+
}
294+
295+
stdin.Close()
296+
297+
if writeErr != nil {
298+
_ = cmd.Process.Kill()
299+
return fmt.Errorf("failed to write to stdin: %w", writeErr)
300+
}
301+
302+
if err := cmd.Wait(); err != nil {
303+
return errors.New("failed to update system password configuration")
179304
}
180305

181306
return nil

accounts1/users/user.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package users
2+
3+
/*
4+
#include <unistd.h>
5+
#include <limits.h>
6+
7+
#ifndef LOGIN_NAME_MAX
8+
#define LOGIN_NAME_MAX 256
9+
#endif
10+
11+
long get_login_name_max() {
12+
long conf = -1;
13+
conf = sysconf(_SC_LOGIN_NAME_MAX);
14+
15+
if (conf == -1) {
16+
conf = LOGIN_NAME_MAX;
17+
}
18+
19+
return conf;
20+
}
21+
*/
22+
import "C"
23+
24+
func LoginNameMaxSize() int {
25+
return int(C.get_login_name_max())
26+
}

0 commit comments

Comments
 (0)