Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PROTOCOL: Identify RTMP client, stack overflow crash, generate core file. #607

Closed
HCLAC opened this issue May 25, 2016 · 4 comments
Closed
Assignees
Labels
Bug It might be a bug. TransByAI Translated by AI/GPT.
Milestone

Comments

@HCLAC
Copy link

HCLAC commented May 25, 2016

Zhao Wenjie:
Jie Ge, when multiple users are watching live broadcasts, there is a problem with the server crashing. After checking the generated core file, it seems that there is a stack overflow. By reading the source code, I found that in the identify_create_stream_client function of the SrsRtmpServer class, it is still recursively calling itself (identify_create_stream_client) in the case of an infinite loop. Personally, I feel that the original intention of this function is: after receiving the createstream command from the client, it should respond to the client and wait for the client to either pull or push the stream. If it is determined what type of client it is, then the mission of this function is completed. However, if it cannot determine the nature of the client, it will keep recursively calling itself, waiting for the client's message command.
Personally, I feel that this has a vulnerability. If a malicious RTMP client, which is properly processing the flow according to the RTMP protocol, continuously sends createstream commands, it will cause the server to crash.
I want to add a parameter to provide a protection mechanism by calculating the number of recursive calls to ensure the normal operation of the server. I don't know if this idea is correct. I hope Jie Ge can guide me when you have time.

TRANS_BY_GPT3

@idxa
Copy link

idxa commented Jul 4, 2016

Can we implement it without using recursion?

TRANS_BY_GPT3

@HCLAC
Copy link
Author

HCLAC commented Jul 8, 2016

Well, I looked at the code again and removed the recursion. It is now implemented with a while loop.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

winlinvip commented Jul 26, 2016

I have never encountered such a malicious client.

TRANS_BY_GPT3

@winlinvip winlinvip added this to the srs 3.0 release milestone Aug 8, 2016
@winlinvip winlinvip changed the title 识别RTMP客户端,栈溢出崩溃,产生core文件 PROTOCOL: 识别RTMP客户端,栈溢出崩溃,产生core文件 Mar 26, 2017
@winlinvip
Copy link
Member

winlinvip commented Jan 26, 2020

Fixed in SRS3, set recursive depth to 3 at

return identify_create_stream_client(dynamic_cast<SrsCreateStreamPacket*>(pkt), stream_id, 3, type, stream_name, duration);

@winlinvip winlinvip self-assigned this Sep 23, 2021
@winlinvip winlinvip added the Bug It might be a bug. label Sep 23, 2021
@winlinvip winlinvip changed the title PROTOCOL: 识别RTMP客户端,栈溢出崩溃,产生core文件 PROTOCOL: Identify RTMP client, stack overflow crash, generate core file. Jul 28, 2023
@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug It might be a bug. TransByAI Translated by AI/GPT.
Projects
None yet
Development

No branches or pull requests

3 participants