security: fix command injection and buffer overflow vulnerabilities#1
Open
devin-ai-integration[bot] wants to merge 1 commit into
Open
security: fix command injection and buffer overflow vulnerabilities#1devin-ai-integration[bot] wants to merge 1 commit into
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
- tools/normalize.py: Replace shell=True subprocess with argument list to prevent command injection via crafted filenames - quick_demo.ipynb: Replace os.system() with subprocess.run() using argument list to prevent shell injection - ffserver.c: Replace all strcpy/strcat with av_strlcpy/av_strlcat to prevent potential buffer overflows in network-facing server code Co-Authored-By: sky_005 <sky_005@126.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses several security vulnerabilities found during a codebase audit:
tools/normalize.py— Eliminates shell command injection. The user-supplied filename (sys.argv[1]) was interpolated into a command string and executed withsubprocess.check_output(..., shell=True). Replaced with a list-based invocation (shell=False), so filenames with shell metacharacters are no longer interpreted.ffserver.c— Replaces 6 instances of unboundedstrcpy/strcatwithav_strlcpy/av_strlcatin the network-facing HTTP server code (start_children,http_parse_request,compute_status,open_input_stream). These prevent potential buffer overflows if source strings ever approach or exceed destination buffer sizes.quick_demo.ipynb— Replacesos.system()withsubprocess.run()using an argument list to eliminate shell injection via glob-derived filenames. (Note: the notebook diff is large due to JSON re-indentation fromjson.dump; the actual code change is a single line.)Review & Testing Checklist for Human
av_strlcatsize argument instart_children(line 513): The third arg must be the total buffer size, not remaining space. The buffer is allocated asslash - my_program_name + sizeof("ffmpeg")(orsizeof("ffmpeg")if no slash). Confirm the size expression matches the allocation.av_strlcpysize arguments incompute_status: The expressionssizeof(sfilename) - (eosf - sfilename)compute remaining space fromeosfto end ofsfilename[1024]. Confirm these are correct given thateosfcan be reassigned viastrrchr.os.system→subprocess.runswap: The diff rewrites the entire notebook due to JSON reformatting. The only semantic change should be in the cell that previously calledos.system('ffmpeg -y -loglevel error -i examples/{} -ar 16000 examples/tmp.wav'.format(ain)).Notes
./configure+makewas run), since the build environment wasn't set up. The functionsav_strlcpy/av_strlcatare declared inlibavutil/avstring.h, whichffserver.calready includes (line 41).Link to Devin session: https://app.devin.ai/sessions/def8bac0843e4323957bd9a44a71cf4f
Requested by: @lien006