Skip to content

Commit d184d3e

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 d184d3e

2 files changed

Lines changed: 162 additions & 13 deletions

File tree

accounts1/users/prop.go

Lines changed: 132 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212
"os"
1313
"os/exec"
14+
"runtime"
1415
"sort"
1516
"strconv"
1617
"strings"
@@ -20,9 +21,7 @@ import (
2021
libdate "github.com/rickb777/date"
2122
)
2223

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

2726
var (
2827
groupFileTimestamp int64 = 0
@@ -157,25 +156,145 @@ func ModifyShell(shell, username string) error {
157156
return doAction(userCmdModify, []string{"-s", shell, username})
158157
}
159158

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

169259
cmd := exec.Command(pwdCmdModify, "-e")
170-
input := fmt.Sprintf("%s:%s\n", username, words)
171-
cmd.Stdin = bytes.NewBufferString(input)
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+
buf := bytes.NewBuffer(make([]byte, 0, len(username)+len(words)+2))
276+
buf.WriteString(username)
277+
buf.WriteString(":")
278+
buf.WriteString(words)
279+
buf.WriteString("\n")
280+
281+
input := buf.Bytes()
282+
_, writeErr := stdin.Write(input)
283+
for i := range input {
284+
input[i] = 0
285+
}
286+
// forbid DCE
287+
runtime.KeepAlive(input)
288+
289+
stdin.Close()
290+
291+
if writeErr != nil {
292+
_ = cmd.Process.Kill()
293+
return fmt.Errorf("failed to write to stdin: %w", writeErr)
294+
}
295+
296+
if err := cmd.Wait(); err != nil {
297+
return errors.New("failed to update system password configuration")
179298
}
180299

181300
return nil

accounts1/users/user.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
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+
func LoginNameMaxSize() int {
29+
return int(C.get_login_name_max())
30+
}

0 commit comments

Comments
 (0)