Repository navigation
Stop crafted SVGs and empty tables from crashing or stalling - #61
Merged
Merged
Conversation
An SVG loaded by <use> or <image> is parsed and checked like the document, attributes included, so deep nesting and entity bombs in it are refused instead of overflowing the stack or raising a bare RuntimeError. <tref> draws through the <use> path: no tref inside its own text, no crash on a missing reference, and the same 20,000-element budget. Embedded SVG images share the budget of the SVG that embeds them and stop at 16 levels, which ends an SVG embedding itself over the web. data: URLs are decoded once per document, an @font-face that is not a font becomes a warning, and a table without cells draws nothing. addressable 2.9.1 is now required: css_parser joins @import paths with it, which took quadratic time on dot segments before that version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VLwjZyBMrd7ZWPbSW6dDXp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An SVG a user uploads could still crash or stall the server that renders it, in the default configuration. Gamba checked the document's own tree for depth and entity expansion, but not an SVG loaded through
<use>or<image>, so adata:URL nested 4,000 levels deep overflowed the stack, and entities in an attribute raised a bareRuntimeError.<tref>had none of the guards<use>has: a tref inside its own text overflowed the stack, one to a missing id raisedNoMethodError, and sixteen levels of two took 38 seconds. Each embedded SVG image also got a fresh<use>budget, so 64 copies of one heavy image took 82 seconds, and withenable_web_requests: truean SVG embedding itself fetched itself 415 times before the stack ran out.table([[]])raisedNoMethodErrortoo.Loaded SVGs now go through the same parse and check as the document, attributes included.
<tref>draws through the<use>path, so it gets the ancestor check and the 20,000-element budget. Embedded SVG images share the budget of the SVG that embeds them and stop at 16 levels.data:URLs are decoded once per document, and an@font-facethat is not a font becomes a warning. A table with no cells draws nothing.test/divergences.ymlwith an oracle example, the SVG ones in the Files from users section of the compatibility page.@importpath with addressable, and before 2.9.1 (GHSA-qq38-m9mg-xjj5) that join took quadratic time on dot segments. A 20 KB import path took 2.1 s on 2.9.0 and 0.2 s on 2.9.2. The gemspec now asks for>= 2.9.1, and the README and guide list addressable as a dependency.svg_security_test.rbraised NameError when an InvalidSVGData test ran first, because Prawn::SVG was not loaded yet; it now requires prawn-svg. The input fuzzer now acceptsPrawn::Errors::CannotFit, which a corrupted advance width can raise just as in Prawn (seed 1234, case 943).Proof
Every new test fails on main and passes here:
test_an_svg_loaded_by_use_is_checked_like_the_document(SystemStackError),test_entities_expanding_too_far_in_an_attribute_are_invalid_svg,test_svg_images_drawn_through_use_share_the_budget_of_their_document,test_svg_images_inside_svg_images_stop_at_a_depth,test_a_data_url_image_drawn_through_use_is_decoded_once(timeout),test_a_tref_inside_the_text_it_refers_to_is_skipped(SystemStackError),test_a_tref_to_a_missing_or_later_text_draws_nothing(NoMethodError),test_trefs_doubling_at_each_level_stop_at_a_budget(timeout),test_a_font_face_that_is_not_a_font_is_left_out(Gamba::TrueType::Error), and the oracle exampletable_without_cells(NoMethodError).test_an_import_climbing_back_up_its_path_resolves_quicklyguards the addressable floor.Reproductions, before then after, Ruby 3.3.6:
<use>copies of an SVG image drawing 20,000 elements: 82 s, then 1.3 s<use>copies of a 480 KB PNG data URL: 11.4 s, then 2.6 s<tref>doubling, 16 levels: 37.7 s, then 1.3 s with a warningBenchmarks against origin/main on Ruby 3.3.6 (a shared 4-core VM, so a few percent is noise).
bench/run.rb, median of 5 alternating processes of 20 renders, main then branch: invoice 17.1 / 17.4 ms without JIT and 16.7 / 16.2 ms with YJIT, report 825 / 826 and 842 / 804 ms, catalog 1,507 / 1,495 and 1,482 / 1,529 ms. All 120 prawn-svg sample SVGs, best of 10 renders in each of 4 alternating processes: 971 / 1,008 ms without JIT and 986 / 1,005 ms with YJIT, with allocations within 0.1% of main. Published numbers are unchanged.bundle exec rake test(corpus fetched, mupdf, poppler and qpdf installed): 1,224 runs, 0 failures, 3 skips (veraPDF and Mustang are not installed here);ORACLE_CANDIDATE=prawn FUZZ_CASES=0gives 1,224 runs, 0 failures.FUZZ_SEED=4242 FUZZ_CASES=500andINPUT_FUZZ_CASES=3000with seeds 777 and 1234 (the seed that failed on main) pass, RuboCop is clean, andrake docs:highlight:checkpasses.