From: Hadi Chokr Date: Thu, 9 Jul 2026 07:30:36 +0000 (+0000) Subject: subids: reject subid file entries that do not fit in id_t X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=db3ca9f917efff9e2dab2fe36fa02fd6ef81ab08;p=thirdparty%2Fshadow.git subids: reject subid file entries that do not fit in id_t range->start and range->count are parsed from /etc/subuid and /etc/subgid as unsigned long, so a corrupt or malicious entry was not bounded by id_t: find_free_range() assigned such a start to intmax_t, which is implementation-defined above INTMAX_MAX, and the range-end computation could exceed INTMAX_MAX. Validate the values where they are parsed, instead of hardening each consumer: bound start to maxof(id_t) in subordinate_parse(), and bound count so that the last ID of the range, start + count - 1, fits in id_t as well; the MIN() caps that bound at maxof(id_t), which also keeps it representable in unsigned long on 32-bit systems. a2ul() reports ERANGE for out-of-range values. Rejected entries are skipped when iterating the database (commonio_next() skips entries whose parse failed), while the raw line is still preserved if the file is rewritten. This protects all consumers of these fields, not just allocation. After this, every visible range satisfies 0 <= start <= start + count - 1 <= (id_t)-1, so the intmax_t arithmetic in find_free_range() is exact and cannot overflow, and oversized values no longer need to be handled there. This is a separate, pre-existing problem from the regression fixed in the previous commit: before de7f1c78f70f8df4b2956f7363da3774348bc58e this arithmetic was done in unsigned long and silently wrapped for such values. Co-authored-by: Dan Anderson Reviewed-by: Alejandro Colomar Signed-off-by: Hadi Chokr --- diff --git a/lib/subordinateio.c b/lib/subordinateio.c index 9602eb55c..af3b2807c 100644 --- a/lib/subordinateio.c +++ b/lib/subordinateio.c @@ -14,6 +14,7 @@ #include "getdef.h" #include "../libsubid/subid.h" #include +#include #include #include #include @@ -113,10 +114,13 @@ subordinate_parse(const char *line) if (streq(fields[2], "")) return NULL; range.owner = fields[0]; - if (str2ul(&range.start, fields[1]) == -1) + if (a2ul(&range.start, fields[1], NULL, 0, 0, maxof(id_t)) == -1) return NULL; - if (str2ul(&range.count, fields[2]) == -1) + if (a2ul(&range.count, fields[2], NULL, 0, 0, + MIN(maxof(id_t) + 1LL - range.start, maxof(id_t))) == -1) + { return NULL; + } return ⦥ }