Skip to content

Commit 45c87a8

Browse files
Watson1978Watson1978
authored
Backport(v1.19) output: enforce strict path boundary validation for tag (#5391)
**Which issue(s) this PR fixes**: Fixes # **What this PR does / why we need it**: This PR enhances the robustness of the `${tag}` placeholder expansion by preventing unintended path boundaries from being crossed. Previously, tags containing relative parent directory patterns (`../`) or absolute paths (e.g., `/etc/passwd`, `\Windows`) were expanded without validation in `extract_placeholders`. This could lead to unexpected file creation or access outside of the intended base directories, especially when using plugins like `out_file`. To address this, we introduced a strict, highly optimized path boundary validation inside `extract_placeholders` specifically for the `${tag}` variable. * By validating the variable before substitution, all core and third-party plugins using `extract_placeholders` automatically benefit from this boundary check. * Backward Compatibility * Tags containing safe slashes (e.g., `app/web`) are still permitted, ensuring URL expansions (e.g., in `out_http`) or safe nested directories remain fully functional. **Docs Changes**: **Release Note**: Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
1 parent 987eeab commit 45c87a8

2 files changed

Lines changed: 145 additions & 1 deletion

File tree

‎lib/fluent/plugin/output.rb‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ class Output < Base
4444
CHUNK_KEY_PLACEHOLDER_PATTERN = /\$\{([-_.@$a-zA-Z0-9]+)\}/
4545
CHUNK_TAG_PLACEHOLDER_PATTERN = /\$\{(tag(?:\[-?\d+\])?)\}/
4646
CHUNK_ID_PLACEHOLDER_PATTERN = /\$\{chunk_id\}/
47+
INVALID_PATH_COMPONENT_PATTERN = %r{\.\.[/\\]|^[/\\]}
48+
PARENT_DIRECTORY_PATTERN = %r{\.\.[/\\]}
4749

4850
CHUNKING_FIELD_WARN_NUM = 4
4951

@@ -825,9 +827,17 @@ def extract_placeholders(str, chunk)
825827
# ${tag}, ${tag[0]}, ${tag[1]}, ... , ${tag[-2]}, ${tag[-1]}
826828
if @chunk_key_tag
827829
if str.include?('${tag}')
830+
if metadata.tag.match?(INVALID_PATH_COMPONENT_PATTERN)
831+
raise Fluent::UnrecoverableError, "Invalid path component detected in tag: #{metadata.tag}"
832+
end
833+
828834
rvalue = rvalue.gsub('${tag}', metadata.tag)
829835
end
830836
if CHUNK_TAG_PLACEHOLDER_PATTERN.match?(str)
837+
if metadata.tag.match?(INVALID_PATH_COMPONENT_PATTERN)
838+
raise Fluent::UnrecoverableError, "Invalid path component detected in tag: #{metadata.tag}"
839+
end
840+
831841
hash = {}
832842
tag_parts = metadata.tag.split('.')
833843
tag_parts.each_with_index do |part, i|
@@ -858,10 +868,21 @@ def extract_placeholders(str, chunk)
858868
end
859869

860870
rvalue = rvalue.gsub(CHUNK_KEY_PLACEHOLDER_PATTERN) do |matched|
861-
hash.fetch(matched) do
871+
replace = hash.fetch(matched) do
862872
log.warn "chunk key placeholder '#{matched[2..-2]}' not replaced. template:#{str}"
863873
''
864874
end
875+
if replace.to_s.match?(INVALID_PATH_COMPONENT_PATTERN)
876+
raise Fluent::UnrecoverableError, "Invalid path component detected in #{matched}: #{replace}"
877+
end
878+
879+
replace
880+
end
881+
# Check if the number of parent directory components (../) has increased due to variable substitution
882+
if rvalue.match?(PARENT_DIRECTORY_PATTERN)
883+
if rvalue.scan(PARENT_DIRECTORY_PATTERN).size > str.scan(PARENT_DIRECTORY_PATTERN).size
884+
raise Fluent::UnrecoverableError, "Invalid path component detected, replaced to: #{rvalue}"
885+
end
865886
end
866887
end
867888

‎test/plugin/test_output.rb‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,129 @@ def waiting(seconds)
448448
assert { logs.any? { |log| log.include?("chunk key placeholder 'key2' not replaced. template:#{tmpl}") } }
449449
end
450450

451+
sub_test_case 'Path boundary validation for ${tag} placeholder' do
452+
data(
453+
'normal_dot' => 'app.web',
454+
'safe_slash' => 'app/web',
455+
'symbol_mixed' => 'my_tag-123',
456+
'no_match' => 'app..web/log',
457+
)
458+
test 'allows valid tags including safe slashes' do |tag|
459+
@i.configure(config_element('ROOT', '', {}, [config_element('buffer', 'tag')]))
460+
t = event_time('2016-04-11 20:30:00 +0900')
461+
v = { key1: "value1" }
462+
m = create_metadata(timekey: t, tag: tag, variables: v)
463+
result = @i.extract_placeholders("/data/${tag}/log", m)
464+
assert_equal "/data/#{tag}/log", result
465+
end
466+
467+
data(
468+
'unix_relative' => '../etc/cron.d',
469+
'windows_relative' => '..\\Windows\\System32',
470+
'unix_absolute' => '/etc/passwd',
471+
'windows_absolute' => '\\Windows'
472+
)
473+
test 'rejects tags containing parent directory relative or absolute root paths' do |tag|
474+
@i.configure(config_element('ROOT', '', {}, [config_element('buffer', 'tag')]))
475+
t = event_time('2016-04-11 20:30:00 +0900')
476+
v = { key1: "value1" }
477+
m = create_metadata(timekey: t, tag: tag, variables: v)
478+
479+
err = assert_raise(Fluent::UnrecoverableError) do
480+
@i.extract_placeholders("/data/${tag}/log", m)
481+
end
482+
assert_match(/Invalid path component detected in tag/, err.message)
483+
end
484+
485+
data(
486+
'unix_relative' => '../etc/cron.d',
487+
'windows_relative' => '..\\Windows\\System32',
488+
'unix_absolute' => '/etc/passwd',
489+
'windows_absolute' => '\\Windows'
490+
)
491+
test 'rejects tags containing traversal paths even when only parts are used' do |tag|
492+
@i.configure(config_element('ROOT', '', {}, [config_element('buffer', 'tag')]))
493+
t = event_time('2016-04-11 20:30:00 +0900')
494+
v = { key1: "value1" }
495+
m = create_metadata(timekey: t, tag: tag, variables: v)
496+
497+
err = assert_raise(Fluent::UnrecoverableError) do
498+
@i.extract_placeholders("/data/${tag[0]}/log", m)
499+
end
500+
assert_match(/Invalid path component detected in tag/, err.message)
501+
end
502+
503+
data(
504+
'unix_relative' => '../etc/cron.d',
505+
'windows_relative' => '..\\Windows\\System32',
506+
'unix_absolute' => '/etc/passwd',
507+
'windows_absolute' => '\\Windows'
508+
)
509+
test 'rejects variables containing parent directory relative or absolute root paths' do |var_value|
510+
@i.configure(config_element('ROOT', '', {}, [config_element('buffer', 'tag,file')]))
511+
t = event_time('2016-04-11 20:30:00 +0900')
512+
v = { file: var_value }
513+
m = create_metadata(timekey: t, tag: 'app.web', variables: v)
514+
515+
err = assert_raise(Fluent::UnrecoverableError) do
516+
@i.extract_placeholders("/data/${tag}/${file}", m)
517+
end
518+
assert_match(/Invalid path component detected in \$\{file\}/, err.message)
519+
end
520+
521+
data(
522+
'combined ../' => { dot: '.', dotslash: './', file: 'safe.log' },
523+
'combined ..\\' => { dot: '.', dotslash: '.\\', file: 'safe.log' },
524+
)
525+
test 'rejects combined variables containing dot and slash combination' do |var_value|
526+
@i.configure(config_element('ROOT', '', {}, [config_element('buffer', 'tag,dot,dotslash,file')]))
527+
t = event_time('2016-04-11 20:30:00 +0900')
528+
v = var_value
529+
m = create_metadata(timekey: t, tag: 'app.web', variables: v)
530+
531+
err = assert_raise(Fluent::UnrecoverableError) do
532+
@i.extract_placeholders("/data/${dot}${dotslash}${file}", m)
533+
end
534+
assert_match(/Invalid path component detected, replaced to/, err.message)
535+
end
536+
537+
test 'passes safely when placeholder expansion does NOT introduce additional ../' do
538+
@i.configure(config_element('ROOT', '', {}, [config_element('buffer', 'val1,val2,val3')]))
539+
t = event_time('2016-04-11 20:30:00 +0900')
540+
v = { val1: "safe", val2: "." }
541+
m = create_metadata(timekey: t, variables: v)
542+
543+
result = @i.extract_placeholders("../data/${val1}${val2}log", m)
544+
assert_equal "../data/safe.log", result
545+
end
546+
547+
data(
548+
'normal_string' => 'app.log',
549+
'safe_slash' => 'logs/app.log',
550+
'integer_val' => 80,
551+
'boolean_val' => true
552+
)
553+
test 'allows valid variables including safe strings and non-string types' do |var_value|
554+
@i.configure(config_element('ROOT', '', {}, [config_element('buffer', 'tag,file')]))
555+
t = event_time('2016-04-11 20:30:00 +0900')
556+
v = { file: var_value }
557+
m = create_metadata(timekey: t, tag: 'app.web', variables: v)
558+
559+
result = @i.extract_placeholders("/data/${tag}/${file}", m)
560+
assert_equal "/data/app.web/#{var_value}", result
561+
end
562+
563+
test 'skips validation if ${tag} placeholder is not used' do
564+
@i.configure(config_element('ROOT', '', {}, [config_element('buffer', 'tag')]))
565+
t = event_time('2016-04-11 20:30:00 +0900')
566+
v = { key1: "value1" }
567+
m = create_metadata(timekey: t, tag: '../etc/passwd', variables: v)
568+
569+
result = @i.extract_placeholders("/data/static/log", m)
570+
assert_equal "/data/static/log", result
571+
end
572+
end
573+
451574
sub_test_case '#placeholder_validators' do
452575
test 'returns validators for time, tag and keys when a template has placeholders even if plugin is not configured with these keys' do
453576
@i.configure(config_element('ROOT', '', {}, [config_element('buffer', '')]))

0 commit comments

Comments
 (0)