Skip to content

Commit 3a9f7fb

Browse files
committed
Land rapid7#3405, improved Nokogiri check for msftidy
2 parents 3a3d038 + 4b97418 commit 3a9f7fb

File tree

1 file changed

+19
-5
lines changed

1 file changed

+19
-5
lines changed

tools/msftidy.rb

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,16 +115,30 @@ def check_shebang
115115
end
116116
end
117117

118+
# Updated this check to see if Nokogiri::XML.parse is being called
119+
# specifically. The main reason for this concern is that some versions
120+
# of libxml2 are still vulnerable to XXE attacks. REXML is safer (and
121+
# slower) since it's pure ruby. Unfortunately, there is no pure Ruby
122+
# HTML parser (except Hpricot which is abandonware) -- easy checks
123+
# can avoid Nokogiri (most modules use regex anyway), but more complex
124+
# checks tends to require Nokogiri for HTML element and value parsing.
118125
def check_nokogiri
119-
msg = "Requiring Nokogiri in modules can be risky, use REXML instead."
126+
msg = "Using Nokogiri in modules can be risky, use REXML instead."
120127
has_nokogiri = false
128+
has_nokogiri_xml_parser = false
121129
@source.each_line do |line|
122-
if line =~ /^\s*(require|load)\s+['"]nokogiri['"]/
123-
has_nokogiri = true
124-
break
130+
if has_nokogiri
131+
if line =~ /Nokogiri::XML\.parse/ or line =~ /Nokogiri::XML::Reader/
132+
has_nokogiri_xml_parser = true
133+
break
134+
end
135+
else
136+
if line =~ /^\s*(require|load)\s+['"]nokogiri['"]/
137+
has_nokogiri = true
138+
end
125139
end
126140
end
127-
error(msg) if has_nokogiri
141+
error(msg) if has_nokogiri_xml_parser
128142
end
129143

130144
def check_ref_identifiers

0 commit comments

Comments
 (0)