libsmb: Avoid undefined behaviour when parsing whoami state

If num_gids is such that the gids array would overflow the rdata buffer,
'p + 8' could produce a result pointing outside the buffer, and thus
result in undefined behaviour. To avoid this, we check num_gids against
the size of the buffer beforehand.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

diff --git a/source3/libsmb/clifsinfo.c b/source3/libsmb/clifsinfo.c
index 135a77f2312322714d8233cf2cbdca55bc086143..8ec74d191beb063c5782d949d76c3898c6f1126a 100644 (file)
--- a/source3/libsmb/clifsinfo.c
+++ b/source3/libsmb/clifsinfo.c
@@ -661,6 +661,13 @@ static void cli_posix_whoami_done(struct tevent_req *subreq)
        state->num_gids = IVAL(rdata, 24);
        state->num_sids = IVAL(rdata, 28);
 
+       /* Ensure the gid array doesn't overflow */
+       if (state->num_gids > (num_rdata - 40) / sizeof(uint64_t)) {
+               tevent_req_nterror(req,
+                       NT_STATUS_INVALID_NETWORK_RESPONSE);
+               return;
+       }
+
        state->gids = talloc_array(state, uint64_t, state->num_gids);
        if (tevent_req_nomem(state->gids, req)) {
                return;
@@ -673,11 +680,6 @@ static void cli_posix_whoami_done(struct tevent_req *subreq)
        p = rdata + 40;
 
        for (i = 0; i < state->num_gids; i++) {
-               if (p + 8 > rdata + num_rdata) {
-                       tevent_req_nterror(req,
-                               NT_STATUS_INVALID_NETWORK_RESPONSE);
-                       return;
-               }
                state->gids[i] = BVAL(p, 0);
                p += 8;
        }