[urvalkheni]

Description

Fix response validation in the shared Pi-hole HTTP helper, Msf::Exploit::Remote::HTTP::Pihole#get_token.

Previously, the response guard was:

return nil unless res or res.code == 200

This caused two issues:

• A nil response could raise an unexpected NoMethodError when accessing res.code.
• Non-200 responses could bypass the guard and proceed to token extraction.

Replace it with:

return nil unless res&.code == 200

This safely handles missing responses and rejects unsuccessful HTTP responses before token extraction.

Added regression tests covering:

• Missing (nil) HTTP responses.
• HTTP 500 responses containing token-like content.

Verification

• Start msfconsole.
• Run: use auxiliary/admin/http/pihole_domains_api_exec
• Set RHOSTS and RPORT against a target (or mock HTTP server).
• Verify that a missing/dropped HTTP response aborts with unexpected-reply: Unable to find token instead of raising NoMethodError.
• Verify that a non-200 response (e.g. HTTP 500) aborts with unexpected-reply: Unable to find token instead of proceeding to token parsing.
• Run: bundle exec rubocop lib/msf/core/exploit/remote/http/pihole.rb spec/lib/msf/core/exploit/http/pihole_spec.rb and confirm 0 offenses.

Results and evidence

Validation was performed against a localhost mock HTTP server. The mock discarded the submitted command and did not execute it.

1. Nil response — before patch

[*] Running module against 127.0.0.1
[-] Auxiliary failed: NoMethodError undefined method `code' for nil
[-] Call stack:
[-]   .../lib/msf/core/exploit/remote/http/pihole.rb:95:in `get_token'
[-]   .../modules/auxiliary/admin/http/pihole_domains_api_exec.rb:107:in `run'

After patch

[*] Running module against 127.0.0.1
[-] Auxiliary aborted due to failure: unexpected-reply: Unable to find token
[*] Auxiliary module execution completed

2. HTTP 500 response containing token string — before patch

[*] Running module against 127.0.0.1
[*] Using token: exampletoken
[*] Sending payload request
[-] Auxiliary aborted due to failure: unexpected-reply: Unable to save settings
[*] Auxiliary module execution completed

After patch

[*] Running module against 127.0.0.1
[-] Auxiliary aborted due to failure: unexpected-reply: Unable to find token
[*] Auxiliary module execution completed

3. Static analysis and test status

$ bundle exec rubocop lib/msf/core/exploit/remote/http/pihole.rb spec/lib/msf/core/exploit/http/pihole_spec.rb

Inspecting 2 files
..

2 files inspected, no offenses detected

RSpec note: 0 examples ran locally because the local test environment requires a configured database.

AI Disclosure

Codex was used for codebase understanding.

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Token retrieval now safely returns no token when the server provides no response or returns an error status, including when the response body contains token-like content.
• Tests
  • Added coverage for missing responses and server errors during token retrieval.

The Pi-hole token method now returns nil when no response is received or the response status is not 200. Specs cover both failure cases.

Changes: Pi-hole token response guard

Layer / File(s)	Summary
Token response guard and specs lib/msf/core/exploit/remote/http/pihole.rb, spec/lib/msf/core/exploit/http/pihole_spec.rb	get_token returns nil for an absent response or a response with status 500, including when its body contains a token. Specs cover both cases.

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: h00die

Merge Risk: Minimal · up to 57b8d

This makes Pi-hole token retrieval fail cleanly when the server gives no response or a non-200 response, instead of raising an error or using a token from a failed reply. Existing modules already handle the nil result. Merge risk is minimal.

Security architecture risk: Minimal · up to 57b8d

The fix rejects unsuccessful responses before token extraction and handles missing responses through the existing failure path. No introduced or worsened architecture-level security risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title "Fix Pi-hole token response validation" matches the changeset. The PR changes the Pi-hole HTTP helper's get_token guard so it returns nil unless a response exists with HTTP status 200, and a…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Pr Description And Verification		The contributor content meets the check's requirements. What changes and why: The description states the old guard, return nil unless res or res.code == 200. It explains two faults. A nil resp…

Full details: Title check

Explanation

The title "Fix Pi-hole token response validation" matches the changeset. The PR changes the Pi-hole HTTP helper's get_token guard so it returns nil unless a response exists with HTTP status 200, and adds specs for nil responses and HTTP 500 responses containing token-like content. The title names the affected component (Pi-hole) and the change (response validation). The title meets the configured requirement to describe the change and the affected component.

Full details: Pr Description And Verification

Explanation

The contributor content meets the check's requirements. What changes and why: The description states the old guard, return nil unless res or res.code == 200. It explains two faults. A nil response raises NoMethodError, and non-200 responses reach token extraction. It gives the new guard, return nil unless res&.code == 200. The raw summary matches this one-line change in pihole.rb. Concrete verification: The description lists reproducible steps. It names the module (auxiliary/admin/http/pihole_domains_api_exec), the options (RHOSTS, RPORT), and the target setup (a localhost mock HTTP server). It also gives the RuboCop command. The unchecked boxes are expected, since reviewers perform those steps. Expected and actual results: The description gives expected outcomes. The module should abort with unexpected-reply: Unable to find token. It shows actual before and after output for both failure cases: - Nil response: before, NoMethodError undefined method 'code' for nil, with a call stack at pihole.rb:95. After, the expected abort message. - HTTP 500 with a token string: before, the run continues to "Sending payload request" and fails with "Unable to save settings". After, the expected abort message. These are original failure outputs for a reproducible bug fix, so the check's requirement for original failure steps is met. The module run output is included, which the check requires for changed module behavior. Static analysis: The RuboCop output reports 0 offenses for both changed files. Limits: The description states that 0 RSpec examples ran locally because a configured database is needed. It also notes that the mock server discarded the submitted command. These are the relevant untested limits. Scope: The check does not require the author to run the module against a real Pi-hole target. The localhost mock evidence is supplied, and the description names it as the test setup. I did not execute any code and did not rely on invented results.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: rapid7/metasploit-framework
• Branch: master
• File: .coderabbit.yaml
• Commit: bea1fb0
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5d2e4469-fe68-45d2-a227-4ca676b11ffa

Commits

Reviewing files that changed from the base of the PR and between bea1fb0 and 57b8d75.

Files selected for processing: 2

• lib/msf/core/exploit/remote/http/pihole.rb
• spec/lib/msf/core/exploit/http/pihole_spec.rb

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.