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 <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

diff --git a/source3/lib/system.c b/source3/lib/system.c
index f1708fa2f4e6907b297547df0c170d807891b9f7..cf4c4d46c36c6382e837a2cb19d5efbd0c5707d3 100644 (file)
--- 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;