On Mon, 20 Feb 2017, Martin Storsjö wrote:
On Mon, 20 Feb 2017, Luca Barbato wrote:
---
libavformat/rtsp.c | 56
+++++++++++++++++++++++++++---------------------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 2c21fa7..6a1a888 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1920,39 +1920,39 @@ static int udp_read_packet(AVFormatContext *s,
RTSPStream **prtsp_st,
struct pollfd *p = rt->p;
int *fds = NULL, fdsnum, fdsidx;
+ if (rt->rtsp_hd) {
+ tcp_fd = ffurl_get_file_handle(rt->rtsp_hd);
+ p[max_p].fd = tcp_fd;
+ p[max_p++].events = POLLIN;
+ } else {
+ tcp_fd = -1;
+ }
+ for (i = 0; i < rt->nb_rtsp_streams; i++) {
+ rtsp_st = rt->rtsp_streams[i];
+ if (rtsp_st->rtp_handle) {
+ if (ret = ffurl_get_multi_file_handle(rtsp_st->rtp_handle,
+ &fds, &fdsnum)) {
+ av_log(s, AV_LOG_ERROR, "Unable to recover rtp ports\n");
+ return ret;
+ }
+ if (fdsnum != 2) {
+ av_log(s, AV_LOG_ERROR,
+ "Number of fds %d not supported\n", fdsnum);
+ return AVERROR_INVALIDDATA;
+ }
+ for (fdsidx = 0; fdsidx < fdsnum; fdsidx++) {
+ p[max_p].fd = fds[fdsidx];
+ p[max_p++].events = POLLIN;
+ }
+ av_free(fds);
+ }
+ }
+
for (;;) {
if (ff_check_interrupt(&s->interrupt_callback))
return AVERROR_EXIT;
if (wait_end && wait_end - av_gettime_relative() < 0)
return AVERROR(EAGAIN);
- max_p = 0;
- if (rt->rtsp_hd) {
- tcp_fd = ffurl_get_file_handle(rt->rtsp_hd);
- p[max_p].fd = tcp_fd;
- p[max_p++].events = POLLIN;
- } else {
- tcp_fd = -1;
- }
- for (i = 0; i < rt->nb_rtsp_streams; i++) {
- rtsp_st = rt->rtsp_streams[i];
- if (rtsp_st->rtp_handle) {
- if (ret = ffurl_get_multi_file_handle(rtsp_st->rtp_handle,
- &fds, &fdsnum)) {
- av_log(s, AV_LOG_ERROR, "Unable to recover rtp
ports\n");
- return ret;
- }
- if (fdsnum != 2) {
- av_log(s, AV_LOG_ERROR,
- "Number of fds %d not supported\n", fdsnum);
- return AVERROR_INVALIDDATA;
- }
- for (fdsidx = 0; fdsidx < fdsnum; fdsidx++) {
- p[max_p].fd = fds[fdsidx];
- p[max_p++].events = POLLIN;
- }
- av_free(fds);
- }
- }
n = poll(p, max_p, POLL_TIMEOUT_MS);
if (n > 0) {
int j = 1 - (tcp_fd == -1);
--
2.9.2
I guess this might be ok.
Theoretically, since we can receive data on the tcp control channel here,
we could actually happen to change the set of streams. But I don't think
that's expected/supported anyway. I guess we mostly need to make sure we
don't accidentally read anything out of bounds. Since the pollfd array is
statically allocated, we should be pretty much safe I think.
Actually I misremembered (now that i read 3/5), that the pollfd array was
preallocated for some maximum number of streams.
Is there any way for the server to trick us into allocating a new stream
here? In that case, this clearly is a security issue, although that issue
already did exist.
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
