diff --git a/resources/asciidoctor/lib/copy_images/copier.rb b/resources/asciidoctor/lib/copy_images/copier.rb index 07d01f02d3064..666ac86ffd85b 100644 --- a/resources/asciidoctor/lib/copy_images/copier.rb +++ b/resources/asciidoctor/lib/copy_images/copier.rb @@ -12,6 +12,8 @@ module CopyImages class Copier include LogUtil + ALLOWED_IMAGE_EXTENSIONS = %w[.png .jpg .jpeg .gif .svg .webp].freeze + def initialize # TODO: store this set on the document so we don't duplicate copies # for sub-documents caused by alternative examples @@ -35,9 +37,30 @@ def copy_image(block, uri) end def perform_copy(block, uri, source) - destination = File.join block.document.options[:to_dir], uri - destination_dir = File.dirname destination - FileUtils.mkdir_p destination_dir + unless ALLOWED_IMAGE_EXTENSIONS.include?(File.extname(uri).downcase) + warn block: block, message: "Refusing to copy non-image file: #{uri}" + return + end + + if File.symlink?(source) + warn block: block, message: "Refusing to copy symlink: #{source}" + return + end + + to_dir = File.expand_path(block.document.options[:to_dir]) + destination = File.expand_path(File.join(to_dir, uri)) + + unless destination.start_with?("#{to_dir}/") + warn block: block, message: "Image path escapes output dir: #{uri}" + return + end + + if File.symlink?(destination) + warn block: block, message: "Destination is a symlink: #{destination}" + return + end + + FileUtils.mkdir_p(File.dirname(destination)) FileUtils.cp source, destination end diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index e658ebcf6646a..9e0f2d38536a9 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -361,6 +361,72 @@ end end + context 'input/output validation' do + let(:tmp) { Dir.mktmpdir } + after(:example) { FileUtils.remove_entry tmp } + + let(:source_file) do + path = File.join(tmp, 'real_image.png') + FileUtils.cp( + File.join(spec_dir, 'resources', 'copy_images', 'example1.png'), + path + ) + path + end + + let(:copier) { CopyImages::Copier.new } + + def make_block(to_dir) + doc = double('document') + allow(doc).to receive(:options).and_return(to_dir: to_dir) + allow(doc).to receive(:attr).with('copy_image').and_return(nil) + block = double('block') + allow(block).to receive(:document).and_return(doc) + allow(block).to receive(:source_location).and_return(nil) + block + end + + context 'when the URI contains path traversal' do + it 'rejects it with a warning and does not write outside to_dir' do + block = make_block(tmp) + outside = File.expand_path(File.join(tmp, '../../../outside.png')) + copier.perform_copy(block, '../../../outside.png', source_file) + expect(File.exist?(outside)).to be false + end + end + + context 'when the URI has a non-image extension' do + it 'rejects it with a warning and does not copy the file' do + block = make_block(tmp) + copier.perform_copy(block, 'evil.yml', source_file) + expect(File.exist?(File.join(tmp, 'evil.yml'))).to be false + end + end + + context 'when the source path is a symlink' do + it 'rejects it with a warning and does not copy' do + symlink_path = File.join(tmp, 'link_image.png') + File.symlink(source_file, symlink_path) + block = make_block(tmp) + copier.perform_copy(block, 'output_image.png', symlink_path) + expect(File.exist?(File.join(tmp, 'output_image.png'))).to be false + end + end + + context 'when the destination is already a symlink' do + it 'rejects it with a warning and does not overwrite the symlink' do + dest_path = File.join(tmp, 'dest_image.png') + # Create a symlink at destination pointing somewhere else + File.symlink('/dev/null', dest_path) + block = make_block(tmp) + copier.perform_copy(block, 'dest_image.png', source_file) + # The symlink should still point to /dev/null, not be replaced + expect(File.symlink?(dest_path)).to be true + expect(File.readlink(dest_path)).to eq('/dev/null') + end + end + end + context 'when the same image is referenced more than once' do let(:input) do <<~ASCIIDOC