fix iovector not checking size before extracting continuous buffer

[NewbieOrange]

The comments for extract_front_continuous and extract_back_continuous indicate they should return nullptr if data are not enough. However this is not checked in code and result in crashes when deserializing invalid or malicious packets.

Need more discussion.

[Coldwings]

The iovector_view version of extract_front_continuous/extract_back_continuous will do the check as line 120 implemented.

    // try to extract `bytes` bytes from front elemnt (iov[0])
    // return the pointer if succeeded, or nullptr otherwise
    void* extract_front_continuous(size_t bytes)
    {
        auto& f = front();
        if (empty() || f.iov_len < bytes)
            return nullptr;

        f.iov_len -= bytes;
        auto rst = f.iov_base;
        (char*&)f.iov_base += bytes;
        if (f.iov_len == 0)
            pop_front();
        return rst;
    }

Both checked if iovector_view is empty or first buffer part is not enough. That is enough for extract_xxx_continuous at all, since all iovector size (.sum()) is always greater-equal to the size of first / last iovec, the condition you added seems not be needed.

The case of lack of buffer leading to crash, could you please add a test case or example to show it?

[NewbieOrange]

@Coldwings the case is when the bytes argument is larger than the entire iovec (and the first iovec, obviously), extract_front_continuous() returns nullptr and then with the following code:

when bytes is very large (>>> system memory limit)

auto buf = do_malloc(bytes);
auto ret = extract_front(bytes, buf);
        return ret == bytes ?
            buf : nullptr;

the above code then crash with segfault when bytes is very large.

The bytes argument is passed from the rpc.h/rpc.cc (not user code), parsed from the request body and then passed to extract_front() unchecked. Since the comment mentioned the function should simply return nullptr on bytes > sum(), I suppose the fix should be done in iovec.

[Coldwings]

@Coldwings the case is when the bytes argument is larger than the entire iovec (and the first iovec, obviously), extract_front_continuous() returns nullptr and then with the following code:

when bytes is very large (>>> system memory limit)

auto buf = do_malloc(bytes);
auto ret = extract_front(bytes, buf);
        return ret == bytes ?
            buf : nullptr;

the above code then crash with segfault when bytes is very large.

The bytes argument is passed from the rpc.h/rpc.cc (not user code), parsed from the request body and then passed to extract_front() unchecked. Since the comment mentioned the function should simply return nullptr on bytes > sum(), I suppose the fix should be done in iovec.

Got it. It seems better to make the condition return just before do_malloc call.

[NewbieOrange]

@Coldwings sure, updated

[Coldwings]

LGTM