ultravideo / uvgRTP

An open-source library for RTP/SRTP media delivery
BSD 2-Clause "Simplified" License
296 stars 84 forks source link

A mistake for APP packet payload copying #197

Closed wowaser closed 1 year ago

wowaser commented 1 year ago

Hello,

there is a mistake in the fuction send_app_packet:

rtp_error_t uvgrtp::rtcp::send_app_packet(const char* name, uint8_t subtype,
    uint32_t payload_len, const uint8_t *payload)
{
    packet_mutex_.lock();

    std::unique_ptr<uint8_t[]> pl = std::make_unique<uint8_t[]>(*payload);
    if (!app_packets_[name].empty())
    {
        UVG_LOG_DEBUG("Adding a new APP packet for sending when %llu packets are waiting to be sent",
            app_packets_[name].size());
    }
    app_packets_[name].emplace_back(name, subtype, payload_len, std::move(pl));
    packet_mutex_.unlock();

    return RTP_OK;
}

Specifically, this line:

std::unique_ptr<uint8_t[]> pl = std::make_unique<uint8_t[]>(*payload);

This creates a unique pointer that stores an empty array with the size of ASCII value of the first character of the payload. So the APP packet payload is always empty.

Moreover, even if it did work, this would not protect the payload from the problem described in #152.

I am also pretty sure that storing the payload in rtcp_app_packet as std::unique_ptr<uint8_t[]> is not that good of an idea.

tampsa commented 1 year ago

Hi, this mistake is fixed by commits bb7c0a1 and aba2fb8.

-Heikki