Hash lookup on frozen constant should untaint the result
When a tainted value (e.g. params[:id]) is used as a key to look up a value from a frozen constant hash, the result is currently treated as tainted. However, the return value can only be one of the hash's constant values (or nil) — user input selects which value is returned but doesn't flow into the result itself.
Example:
class MailtemplatesController < ApplicationController
TEMPLATE_IDS = %w[
layout message_available password_reset
].index_by(&:itself).freeze
def set_template_id
@template_id = TEMPLATE_IDS[params[:id]]
end
def edit
# Brakeman flags this as "Parameter value used in file name"
# but @template_id can only be "layout", "message_available",
# "password_reset", or nil — never arbitrary user input.
File.read(File.join(@template_path, "#{@template_id}.html"))
end
endBrakeman reports this as a File Access warning because it traces taint from params[:id] through the hash lookup and instance variable assignment. But TEMPLATE_IDS[anything] can only return a value that existed in the hash at freeze time — the tainted key cannot influence what string is returned, only which one.
Suggested behavior:
For Hash#[] and Hash#fetch where the receiver is a frozen constant defined at the class/module level with all-literal values, the return value should not carry taint from the key.
Edge cases to consider:
HASH.fetch(tainted) — safe (raises KeyError on miss)
HASH.fetch(tainted) { block } — block return needs its own taint analysis
HASH.fetch(tainted, default) — only safe if default is also untainted
Non-frozen or non-constant hashes should continue to propagate taint
Workaround:
Currently using brakeman.ignore for these false positives. The pattern of "validate input against a constant allowlist, then use the looked-up value" is common enough that supporting it would reduce ignore-file noise.
I can possibly submit a PR for this change but wanted to open this first to make sure it's agreed on as a good idea before writing anything. Thank you.
Source: presidentbeef/brakeman