Fix SNMP MIB eval/shell injection, ZIP path traversal, and RPC pre-auth resource exhaustion - #21869
Open
prithvee07 wants to merge 3 commits into
Open
Fix SNMP MIB eval/shell injection, ZIP path traversal, and RPC pre-auth resource exhaustion#21869prithvee07 wants to merge 3 commits into
prithvee07 wants to merge 3 commits into
Conversation
SNMP::MIB.import_module interpolated a caller-controlled MIB filename into a shell command and then eval'd the smidump converter output as Ruby, letting an attacker-influenced MIB file execute arbitrary shell and Ruby code. Run smidump via Open3.popen3 with an argv array instead of a shell string, and replace the eval with a schema-validating parser (SNMP::PythonLiteralParser) for the primitive Python-literal subset smidump emits. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YABLbiZQ77C8DtgjjfmR3R
is_child_of? authorized extraction destinations with a raw target.start_with?(target_dir) string check, so a sibling directory whose name merely starts with the extraction root's name (e.g. "<tmp>_evil/file") passed containment even though it isn't a descendant. Compare expanded paths for exact equality or require the extraction root followed by a path separator. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YABLbiZQ77C8DtgjjfmR3R
msfrpcd passed the full unauthenticated HTTP request body to MessagePack.unpack before any size limit or token authentication was applied, letting an unauthenticated client force large allocation/parsing work on the default 0.0.0.0 listener. Enforce a 10MB ceiling in the HTTP packet parser (checked against Content-Length before the body is buffered, and against the decoded chunked body size) and re-check body size in Msf::RPC::Service#process before handing it to MessagePack.unpack as defense in depth. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YABLbiZQ77C8DtgjjfmR3R
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
1 participant
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
Fixes three medium-severity findings from an internal security scan of this repo:
SNMP::MIB.import_moduleinterpolated a caller-controlled filename into a shell command andeval'd the smidump converter output. Now runssmidumpviaOpen3.popen3with an argv array and parses the output with a newSNMP::PythonLiteralParserinstead ofeval.is_child_of?used a rawstart_with?string check, so a sibling directory like<tmp>_evil/passed containment. Now compares expanded paths for exact equality or requires the extraction root followed by a path separator.msfrpcddecoded the full unauthenticated request body withMessagePack.unpackbefore any size limit or auth check. Added a 10MB ceiling enforced in the HTTP packet parser (before the body is buffered) and re-checked inMsf::RPC::Service#processbefore unpacking.Each fix has an accompanying test (
spec/lib/snmp/mib_spec.rb,spec/support/shared/examples/msf/db_manager/import/metasploit_framework/zip.rb,spec/lib/rex/proto/http/packet_spec.rb,spec/lib/msf/core/rpc/v10/service_spec.rb).Test plan
SNMP::PythonLiteralParserdirectly against real MIB-shaped input and injected backtick/method-call payloads that the previousevalwould have executedis_child_of?directly: accepts root/descendants, rejects sibling-prefix pathsMessagePack.unpackbundle exec rspecrun (blocked in dev environment: no network, vendored gems incomplete — CI should run the added specs)🤖 Generated with Claude Code
https://claude.ai/code/session_01YABLbiZQ77C8DtgjjfmR3R