From a4e25857d96ce7009dcb89162ddf7cc47dbfa364 Mon Sep 17 00:00:00 2001
From: Matthew Flanagan <188046+mattimustang@users.noreply.github.com>
Date: Fri, 5 Jun 2026 16:05:42 +1000
Subject: [PATCH] fix: mitigate ReDoS in /nodes/conf_search (CWE-1333)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Regexp.new was called directly on unsanitised user input with no error
handling or timeout, allowing an authenticated attacker to supply a
pathological pattern that hangs a Puma worker thread indefinitely, or
an invalid pattern that raises an unhandled exception.

- Wrap Regexp.new in rescue RegexpError → HTTP 400 for invalid patterns
- Wrap each per-node config match in Timeout.timeout(2) → skip node on
  timeout, preventing a crafted regex from exhausting the thread pool
- Add tests covering invalid pattern, no match, match, and timeout cases
---
 CHANGELOG.md                       |  3 ++
 lib/oxidized/web/webapp.rb         | 15 +++++++-
 spec/web/nodes/conf_search_spec.rb | 61 ++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 2 deletions(-)
 create mode 100644 spec/web/nodes/conf_search_spec.rb

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 251168b..098a25a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 ### Changed
 
 ### Fixed
+- Fix ReDoS vulnerability in `/nodes/conf_search`: invalid patterns now return HTTP 400 instead
+  of an unhandled exception, and per-node regex evaluation is wrapped in a 2-second timeout
+  so a crafted pattern cannot hang a Puma worker thread indefinitely (CWE-1333)
 
 
 ## [0.18.1 – 2026-01-19]
diff --git a/lib/oxidized/web/webapp.rb b/lib/oxidized/web/webapp.rb
index 5e69e3d..d887639 100644
--- a/lib/oxidized/web/webapp.rb
+++ b/lib/oxidized/web/webapp.rb
@@ -4,6 +4,7 @@
 require 'tilt/haml'
 require 'htmlentities'
 require 'charlock_holmes'
+require 'timeout'
 module Oxidized
   module API
     require 'oxidized/web/version'
@@ -65,13 +66,23 @@ class WebApp < Sinatra::Base
       end
 
       post '/nodes/conf_search.?:format?' do
-        @to_research = Regexp.new params[:search_in_conf_textbox]
+        begin
+          @to_research = Regexp.new(params[:search_in_conf_textbox])
+        rescue RegexpError => e
+          halt 400, "Invalid regular expression: #{e.message}"
+        end
         nodes_list = nodes.list.map
         @nodes_match = []
         nodes_list.each do |n|
           node, @json = route_parse n[:name]
           config = nodes.fetch node, n[:group]
-          @nodes_match.push({ node: n[:name], full_name: n[:full_name] }) if config[@to_research]
+          begin
+            Timeout.timeout(2) do
+              @nodes_match.push({ node: n[:name], full_name: n[:full_name] }) if config[@to_research]
+            end
+          rescue Timeout::Error
+            next
+          end
         end
         @data = @nodes_match
         out :conf_search
diff --git a/spec/web/nodes/conf_search_spec.rb b/spec/web/nodes/conf_search_spec.rb
new file mode 100644
index 0000000..8db60f9
--- /dev/null
+++ b/spec/web/nodes/conf_search_spec.rb
@@ -0,0 +1,61 @@
+require_relative '../../spec_helper'
+require 'json'
+
+describe Oxidized::API::WebApp do
+  include Rack::Test::Methods
+
+  def app
+    Oxidized::API::WebApp
+  end
+
+  before do
+    @nodes = mock('Oxidized::Nodes')
+    app.set(:nodes, @nodes)
+  end
+
+  describe 'POST /nodes/conf_search' do
+    it 'returns 400 for an invalid regular expression' do
+      post '/nodes/conf_search', search_in_conf_textbox: '[invalid('
+
+      _(last_response.status).must_equal 400
+      _(last_response.body).must_include 'Invalid regular expression'
+    end
+
+    it 'returns an empty list when no configs match' do
+      @nodes.expects(:list).returns([{ name: 'sw1', full_name: 'Switch 1' }])
+      @nodes.expects(:fetch).with('sw1', nil).returns('hostname sw1')
+
+      post '/nodes/conf_search.json', search_in_conf_textbox: 'interface'
+
+      _(last_response.ok?).must_equal true
+      _(JSON.parse(last_response.body)).must_equal []
+    end
+
+    it 'returns nodes whose configs match the pattern' do
+      @nodes.expects(:list).returns(
+        [{ name: 'sw1', full_name: 'Switch 1' },
+         { name: 'sw2', full_name: 'Switch 2' }]
+      )
+      @nodes.expects(:fetch).with('sw1', nil).returns("hostname sw1\ninterface GigabitEthernet0/0")
+      @nodes.expects(:fetch).with('sw2', nil).returns('hostname sw2')
+
+      post '/nodes/conf_search.json', search_in_conf_textbox: 'interface'
+
+      _(last_response.ok?).must_equal true
+      result = JSON.parse(last_response.body)
+      _(result.length).must_equal 1
+      _(result.first['node']).must_equal 'sw1'
+    end
+
+    it 'skips nodes that exceed the search timeout and still returns 200' do
+      @nodes.expects(:list).returns([{ name: 'sw1', full_name: 'Switch 1' }])
+      @nodes.expects(:fetch).with('sw1', nil).returns('hostname sw1')
+      Timeout.stubs(:timeout).raises(Timeout::Error)
+
+      post '/nodes/conf_search.json', search_in_conf_textbox: 'hostname'
+
+      _(last_response.ok?).must_equal true
+      _(JSON.parse(last_response.body)).must_equal []
+    end
+  end
+end