Date: Thu, 17 Aug 2017 21:33:18 +0000 From: "Wall, Stephen" <swall@redcom.com> To: "freebsd-bugs@freebsd.org" <freebsd-bugs@freebsd.org> Subject: Possible bug with account/password expiration Message-ID: <5320af8198d943f69b688d78203d8e31@exch-02.redcom.com>
next in thread | raw e-mail | index | archive | help
While trying to determine the cause of a problem a customer had with being =
unable to reset their root password, I came across this piece of code in us=
r.sbin/pwd_mkdb/pwd_mkdb.c:
#define SCALAR(e) store =3D htonl((uint32_t)(e)); \
memmove(p, &store, sizeof(store)); \
p +=3D sizeof(store);
#define LSCALAR(e) store =3D HTOL((uint32_t)(e)); \
memmove(p, &store, sizeof(store)); \
p +=3D sizeof(store);
#define HTOL(e) (openinfo.lorder =3D=3D BYTE_ORDER ? \
(uint32_t)(e) : \
bswap32((uint32_t)(e)))
if (!is_comment &&=20
(!username || (strcmp(username, pwd.pw_name) =3D=3D 0))) {
/* Create insecure data. */
p =3D buf;
COMPACT(pwd.pw_name);
COMPACT("*");
SCALAR(pwd.pw_uid);
SCALAR(pwd.pw_gid);
SCALAR(pwd.pw_change);
COMPACT(pwd.pw_class);
COMPACT(pwd.pw_gecos);
COMPACT(pwd.pw_dir);
COMPACT(pwd.pw_shell);
SCALAR(pwd.pw_expire);
SCALAR(pwd.pw_fields);
data.size =3D p - buf;
Note the cast to uint32_t in the SCALAR macro, then the use of that macro f=
urther down with pwd.pw_change and pwd.pw_expire. These fields are declare=
d as time_t, which is a 64-bit value on x86. It seems to me this will incor=
rectly truncate the values when passing them to htonl().
Am I missing something here?
On a side note, use of these fields is pretty inconsistent throughout the c=
ode. They are passed around variously as time_t, intmax_t, and long. Whil=
e these do happen to all be the same size on x86 (I did not investigate oth=
er platforms), it's not good practice and could lead to problems if these t=
ypes diverge.
Thanks
-spw
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5320af8198d943f69b688d78203d8e31>