From: Uri Simchoni <uri@samba.org>
Date: Mon, 19 Sep 2016 15:24:58 +0000 (+0300)
Subject: cliquota: some security hardening
X-Git-Url: http://git.samba.org/?p=metze%2Fsamba%2Fwip.git;a=commitdiff_plain;h=72c773c9c29d5971d3e8403a11937323997fa0b0

cliquota: some security hardening

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>
---

diff --git a/source3/libsmb/cliquota.c b/source3/libsmb/cliquota.c
index 84028bbfd8d6..e4589c47655e 100644
--- a/source3/libsmb/cliquota.c
+++ b/source3/libsmb/cliquota.c
@@ -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: