hardenedbsd/HardenedBSD
1
0
Fork 0
mirror of https://git.hardenedbsd.org/hardenedbsd/HardenedBSD.git synced 2024-11-21 18:50:50 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

cred: kern_setgroups(): Internally use int as number of groups' type

Browse Source 
sys_setgroups() (and sys_getgroups()) was changed in commit "kern: fail
getgroup and setgroup with negative int" (4bc2174a1b) to take the
number of groups as an 'int' (for sys_getgroups(), POSIX mandates this
change; for sys_setgroups(), which it does not standardize, it's
arguably for consistency).

All our internal APIs related to groups on 'struct ucred', as well as
related members on the latter, treat that number as an 'int' as well
(and not a 'u_int').

Consequently, to avoid surprises, change kern_setgroups() to behave the
same, and fix audit_arg_groupset() accordingly.  With that change,
everything is handled with signed integers internally.

Update sanity checks accordingly.

Reviewed by:    mhorne
Approved by:    markj (mentor)
MFC after:      3 days
Differential Revision:  https://reviews.freebsd.org/D46912
This commit is contained in:
Olivier Certner 2024-10-01 18:46:46 +02:00
parent 664b9fcb1c
commit abd39811cd
No known key found for this signature in database
GPG Key ID: 8CA13040971E2627
4 changed files with 20 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
sys/kern/kern_prot.c
View File

@ -815,6 +815,15 @@ sys_setgroups(struct thread *td, struct setgroups_args *uap)
gid_t *groups;
int gidsetsize, error;
/*
* Sanity check size now to avoid passing too big a value to copyin(),
* even if kern_setgroups() will do it again.
*
* Ideally, the 'gidsetsize' argument should have been a 'u_int' (and it
* was, in this implementation, for a long time), but POSIX standardized
* getgroups() to take an 'int' and it would be quite entrapping to have
* setgroups() differ.
*/
gidsetsize = uap->gidsetsize;
if (gidsetsize > ngroups_max + 1 || gidsetsize < 0)
return (EINVAL);
@ -843,13 +852,16 @@ gidp_cmp(const void *p1, const void *p2)
}
int
kern_setgroups(struct thread *td, u_int ngrp, gid_t *groups)
kern_setgroups(struct thread *td, int ngrp, gid_t *groups)
{
struct proc *p = td->td_proc;
struct ucred *newcred, *oldcred;
int error;
MPASS(ngrp <= ngroups_max + 1);
/* Sanity check size. */
if (ngrp < 0 || ngrp > ngroups_max + 1)
return (EINVAL);
AUDIT_ARG_GROUPSET(groups, ngrp);
newcred = crget();
crextend(newcred, ngrp);

2
sys/security/audit/audit.h
View File

@ -98,7 +98,7 @@ void audit_arg_rgid(gid_t rgid);
void audit_arg_ruid(uid_t ruid);
void audit_arg_sgid(gid_t sgid);
void audit_arg_suid(uid_t suid);
void audit_arg_groupset(gid_t *gidset, u_int gidset_size);
void audit_arg_groupset(gid_t *gidset, int gidset_size);
void audit_arg_login(char *login);
void audit_arg_ctlname(int *name, int namelen);
void audit_arg_mask(int mask);

8
sys/security/audit/audit_arg.c
View File

@ -263,13 +263,13 @@ audit_arg_suid(uid_t suid)
}
void
audit_arg_groupset(gid_t *gidset, u_int gidset_size)
audit_arg_groupset(gid_t *gidset, int gidset_size)
{
u_int i;
int i;
struct kaudit_record *ar;
KASSERT(gidset_size <= ngroups_max + 1,
("audit_arg_groupset: gidset_size > (kern.ngroups + 1)"));
KASSERT(gidset_size >= 0 && gidset_size <= ngroups_max + 1,
("audit_arg_groupset: gidset_size < 0 or > (kern.ngroups + 1)"));
ar = currecord();
if (ar == NULL)

2
sys/sys/syscallsubr.h
View File

@ -320,7 +320,7 @@ int kern_select(struct thread *td, int nd, fd_set *fd_in, fd_set *fd_ou,
fd_set *fd_ex, struct timeval *tvp, int abi_nfdbits);
int kern_sendit(struct thread *td, int s, struct msghdr *mp, int flags,
struct mbuf *control, enum uio_seg segflg);
int kern_setgroups(struct thread *td, u_int ngrp, gid_t *groups);
int kern_setgroups(struct thread *td, int ngrp, gid_t *groups);
int kern_setitimer(struct thread *, u_int, struct itimerval *,
struct itimerval *);
int kern_setpriority(struct thread *td, int which, int who, int prio);