cliquota: some security hardening
authorUri Simchoni <uri@samba.org>
Mon, 19 Sep 2016 15:24:58 +0000 (18:24 +0300)
committerJeremy Allison <jra@samba.org>
Tue, 4 Oct 2016 00:00:23 +0000 (02:00 +0200)
Add some checks for validity of the offset in
the return buffer.

Signed-off-by: Uri Simchoni <uri@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
source3/libsmb/cliquota.c

index 84028bbfd8d61cccc45220ed6e5733cd9e341a43..e4589c47655e44c230e8d62144e49dda464ed06f 100644 (file)
@@ -73,11 +73,18 @@ bool parse_user_quota_record(const uint8_t *rdata,
 
        /* sid len */
        sid_len = IVAL(rdata,4);
+       if (40 + sid_len < 40) {
+               return false;
+       }
 
        if (rdata_count < 40+sid_len) {
                return False;           
        }
 
+       if (*offset != 0 && *offset < 40 + sid_len) {
+               return false;
+       }
+
        /* unknown 8 bytes in pdata 
         * maybe its the change time in NTTIME
         */
@@ -260,10 +267,9 @@ static NTSTATUS cli_list_user_quota_step(struct cli_state *cli,
                status = NT_STATUS_NO_MORE_ENTRIES;
        }
 
-       offset = 1;
-       for (curdata=rdata,curdata_count=rdata_count;
-               ((curdata)&&(curdata_count>=8)&&(offset>0));
-               curdata +=offset,curdata_count -= offset) {
+       curdata = rdata;
+       curdata_count = rdata_count;
+       while (true) {
                ZERO_STRUCT(qt);
                if (!parse_user_quota_record((const uint8_t *)curdata, curdata_count,
                                             &offset, &qt)) {
@@ -286,6 +292,25 @@ static NTSTATUS cli_list_user_quota_step(struct cli_state *cli,
                tmp_list_ent->mem_ctx = mem_ctx;                
 
                DLIST_ADD((*pqt_list),tmp_list_ent);
+
+               if (offset > curdata_count) {
+                       DEBUG(1, ("out of bounds offset in quota record\n"));
+                       status = NT_STATUS_INVALID_NETWORK_RESPONSE;
+                       goto cleanup;
+               }
+
+               if (curdata + offset < curdata) {
+                       DEBUG(1, ("Pointer overflow in quota record\n"));
+                       status = NT_STATUS_INVALID_NETWORK_RESPONSE;
+                       goto cleanup;
+               }
+
+               curdata += offset;
+               curdata_count -= offset;
+
+               if (offset == 0) {
+                       break;
+               }
        }
 
 cleanup: