V2 decoder overflow RCE fix

[guidovranken]

No description provided.

[guidovranken]

Per this rule in the spec

When two pointers are subtracted, both shall point to elements of the same array object, or one past the last element of the array object;
the result is the difference of the subscripts of the two array elements. 

the following ought to be legal pointer arithmetic:

msg_size_ > allocator.data () + allocator.size () - read_pos_

because

allocator.data () + allocator.size () constitutes a valid pointer, and subtracting the valid pointer read_pos_ from the result, yields the "difference of the subscripts of the two array elements", which is the distance in bytes for the type unsigned char.

To guarantee this in all circumstances, we might want to add a static_assert (or similar) that the pointer types of allocator.data () and read_pos_ are equivalent, and moreover point to a type of size 1.

Furthermore, this assumes read_pos_ >= allocator.data (), for which we might want to add an assert(), as crashing via assert() is less severe than potential remote code execution resulting from an invalid pointer computation.

[sigiesec]

Thanks for the fix! Unfortunately, this fails to compile with some builds, e.g. here: https://travis-ci.org/zeromq/libzmq/jobs/477075295#L1215
A cast to ptrdiff_t might be added to resolve this.

[sigiesec]

Fixes #3351

[bluca]

@guidovranken thank you very much for finding the bug and sending the fix. There's a formatting error but I'll take care of that, as you've done enough already.

I'll try and do a new release soon-ish, as soon as I can do the validation and changelog.

[guidovranken]

@guidovranken thank you very much for finding the bug and sending the fix. There's a formatting error but I'll take care of that, as you've done enough already.

I'll try and do a new release soon-ish, as soon as I can do the validation and changelog.

Oh, I just pushed the fix for formatting before I saw your message.