[PATCH] More ldb/tdb tests.

[Jeremy Allison]

On Fri, Nov 06, 2015 at 06:34:25PM +1300, Andrew Bartlett wrote:
> 
> Here is the patch set.  I've reviewed them, and made that clarifying
> comment.  Sadly we don't get the performance benefit until we get other
> (also unsigned!) patches from matthieu's ldb-perfs branch.

I'd like some changes here please.

+static bool ldb_consume_element_data(uint8_t **pp, unsigned int *premaining)
+{
+       unsigned int remaining = *premaining;
+       uint8_t *p = *pp;
+       uint32_t num_values = pull_uint32(p, 0);
+       uint32_t len;
+       int j;
+
+       p += 4;
+       remaining -= 4;
+       for (j=0; j < num_values; j++) {
+               len = pull_uint32(p, 0);
+               if (len > remaining - 5) {
+                       return false;
+               }
+               remaining -= len + 4 + 1;
+               p += len + 4 + 1;
+       }
+
+       *premaining = remaining;
+       *pp = p;
+       return true;
+}

I'd prefer every pull from *pp be range checked against
remaining for safety's sake. Also remember every addition
from a pulled value also must be integer wrap checked.
So make it something like:

static bool ldb_consume_element_data(uint8_t **pp, unsigned int *premaining)
{
	unsigned int remaining = *premaining;
	uint8_t *p = *pp;
	uint32_t num_values;
	uint32_t j;

	if (remaining < 4) {
		return false;
	}

	num_values = pull_uint32(p, 0);
	p += 4;
	remaining -= 4;

	for (j=0; j < num_values; j++) {
		uint32_t len;

		if (remaining < 4) {
			return false;
		}
		len = pull_uint32(p, 0);
		p += 4;
		remaining -= 4;

		if (len + 1 < len) {
			/* Integer wrap. */
			return false;
		}
		len += 1;
		if (remaining < len)
			return false;
		}
		remaining -= len;
		p += len;
	}

	*premaining = remaining;
	*pp = p;
	return true;
}

Also the code changes inside ldb_unpack_data()
require the same paranoia.

These:

+                       if (!keep) {
+                               remaining -= len + 1;
+                               p += len + 1;

and these:

+               element->flags = 0;
                remaining -= len + 1;
                p += len + 1;

require the range + wrap check paranoia tests.

Jeremy.