Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
## [Unreleased]

### Added
- Security regression tests for XSS prevention in node name, version, and diff views (@mattimustang)

### Changed

### Fixed
- Fix XSS vulnerability (CWE-79) by enabling HAML's escape_html globally; user-controlled values in node names, group names, model names, and URL parameters are now HTML-escaped in all templates (@mattimustang)


## [0.18.1 – 2026-01-19]
Expand Down
2 changes: 1 addition & 1 deletion lib/oxidized/web/views/layout.haml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@
%i.bi.bi-moon-fill

.container-fluid
=yield
!= yield
!=haml :footer

2 changes: 1 addition & 1 deletion lib/oxidized/web/views/node.haml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@
%i.bi.bi-repeat
%pre.bg-body-tertiary.border.border-secondary-subtle.rounded
%code
=escape_once(JSON.pretty_generate(@data))
!= escape_once(JSON.pretty_generate(@data))


2 changes: 1 addition & 1 deletion lib/oxidized/web/webapp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module API
class WebApp < Sinatra::Base
helpers Sinatra::UrlForHelper
set :public_folder, proc { File.join(root, 'public') }
set :haml, { escape_html: false }
set :haml, { escape_html: true }

get '/' do
redirect url_for('/nodes')
Expand Down
71 changes: 71 additions & 0 deletions spec/web/xss_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
require_relative '../spec_helper'
require 'cgi'

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 'reflected XSS prevention' do
it 'escapes node name in /node/version versions list' do
# Use a payload without '/' so the routing does not split on rpartition('/')
malicious = '<img src=x onerror=alert(1)>'
@nodes.expects(:version).with(malicious, nil).returns([])

get "/node/version?node_full=#{CGI.escape(malicious)}"

_(last_response.ok?).must_equal true
_(last_response.body).must_include('&lt;img src=x onerror=alert(1)&gt;')
_(last_response.body).wont_include('<img src=x onerror=alert(1)>')
end

it 'escapes node name in /node/version/view' do
malicious = '<script>alert(1)</script>'
@nodes.expects(:get_version).with(malicious, '', 'abc123').returns('device config')

get "/node/version/view?node=#{CGI.escape(malicious)}&group=&oid=abc123&epoch=0&num=1"

_(last_response.ok?).must_equal true
_(last_response.body).must_include('&lt;script&gt;alert(1)&lt;/script&gt;')
_(last_response.body).wont_include('<script>alert(1)</script>')
end

it 'escapes node name in /node/version/diffs' do
malicious = '<script>alert(1)</script>'
versions = [{ oid: 'C006', time: Time.parse('2025-02-05 19:49:00 +0100') }]
@nodes.expects(:version).with(malicious, nil).returns(versions)
@nodes.expects(:get_diff).with(malicious, '', 'C006', nil).returns(
{ patch: "- old line\n+ new line\n", stat: [1, 1] }
)

get "/node/version/diffs?node=#{CGI.escape(malicious)}&group=&oid=C006&epoch=0&num=1"

_(last_response.ok?).must_equal true
_(last_response.body).must_include('&lt;script&gt;alert(1)&lt;/script&gt;')
_(last_response.body).wont_include('<script>alert(1)</script>')
end
end

describe 'stored XSS prevention' do
it 'escapes node names in /nodes list' do
malicious = '<script>alert(1)</script>'
@nodes.expects(:list).returns(
[{ name: malicious, ip: '10.0.0.1', model: 'ios',
full_name: malicious, group: 'default', time: Time.now, mtime: Time.now }]
)

get '/nodes'

_(last_response.ok?).must_equal true
_(last_response.body).must_include('&lt;script&gt;alert(1)&lt;/script&gt;')
_(last_response.body).wont_include('<script>alert(1)</script>')
end
Comment on lines +64 to +124
end
end