From: Volker Lendecke Date: Fri, 30 Oct 2020 10:04:21 +0000 (+0100) Subject: lib: Fix a theoretical out-of-bounds write X-Git-Tag: samba-4.14.0rc1~677 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=30c917c215c627a8b306c4e245f9443a3b29606d;p=thirdparty%2Fsamba.git lib: Fix a theoretical out-of-bounds write This routine looked fishy: We do cap_vals[num_cap_vals++] = XXX based on #ifdefs and capabilities. Then later on we did a check that we did not overwrite the stack. The change I did is to just count the number of num_cap_vals++, right now it's 5. I know it is in different switch branches, but with the #ifdefs it's a bit clumsy to read the exact number of actual num_cap_vals++ that can happen in one run. On debian buster, cap_val_t is an int, so this is not really wasting too much. Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison --- diff --git a/source3/lib/system.c b/source3/lib/system.c index f1708fa2f4e..cf4c4d46c36 100644 --- a/source3/lib/system.c +++ b/source3/lib/system.c @@ -587,7 +587,8 @@ char *sys_getwd(void) static bool set_process_capability(enum smbd_capability capability, bool enable) { - cap_value_t cap_vals[2] = {0}; + /* "5" is the number of "num_cap_vals++" below */ + cap_value_t cap_vals[5] = {0}; size_t num_cap_vals = 0; cap_t cap; @@ -613,6 +614,12 @@ static bool set_process_capability(enum smbd_capability capability, } switch (capability) { + /* + * WARNING: If you add any #ifdef for a fresh + * capability, bump up the array size in the + * declaration of cap_vals[] above just to be + * trivially safe to never overwrite cap_vals[]. + */ case KERNEL_OPLOCK_CAPABILITY: #ifdef CAP_NETWORK_MGT /* IRIX has CAP_NETWORK_MGT for oplocks. */ @@ -639,8 +646,6 @@ static bool set_process_capability(enum smbd_capability capability, #endif } - SMB_ASSERT(num_cap_vals <= ARRAY_SIZE(cap_vals)); - if (num_cap_vals == 0) { cap_free(cap); return True;