rtmpproto: Avoid rare crashes in the fail: codepath in rtmp_open

mstorsjo · mstorsjo · commit ac494b8325cc · 2025-02-06T23:47:41.000+02:00
When running the cleanup in rtmp_close on failures in rtmp_open, we can in rare cases end up using rt->playpath, assuming that it is still set. The crash could happen if we hit the fail codepath in rtmp_open while publishing (rt->is_input == 0) with rt->state set to a value > STATE_FCPUBLISH. This would normally not happen while publishing; either we have an error (and rt->state <= STATE_FCPUBLISH) or we reach rt->state = STATE_PUBLISHING, and then we also return successfully from rtmp_open. The unexpected combination of states could happen if the server responds with e.g. "NetStream.Play.Stop" while expecting "NetStream.Publish.Start"; this sets rt->state to STATE_STOPPED, which also fulfills the condition "> STATE_FCPUBLISH". We don't need to free the rt->playpath/tcurl/flashver strings here; they're handled via AVOption, and thus are freed automatically when the protocol instance is freed (that's why they aren't freed manually within the rtmp_close function either). We also don't need to free the AVDictionary with options; it's owned by the caller. A smaller fix would be to just call rtmp_close before freeing the strings and dictionary, but as we don't need to free them at all, let's remove that redundant code. Signed-off-by: Martin Storsjö <martin@martin.st> (cherry picked from commit 8f4819c)
diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
@@ -2918,10 +2918,6 @@ static int rtmp_open(URLContext *s, const char *uri, int flags, AVDictionary **o
     return 0;
 
 fail:
-    av_freep(&rt->playpath);
-    av_freep(&rt->tcurl);
-    av_freep(&rt->flashver);
-    av_dict_free(opts);
     rtmp_close(s);
     return ret;
 }
