1.5K input consumes 8Gb of RAM

[kcc]

found by libFuzzer, see #139

Feed the attached input to the fuzzer target function, observe it consume 8Gb RAM.
crash-3511ff5c1647150595846ac414c595cccac34f18.pdf

Huge allocations seem to be coming from here:

    #10 0x4e9a07 in hb_buffer_t::enlarge(unsigned int) src/hb-buffer.cc:110:37
    #11 0x4ed70a in hb_buffer_t::ensure(unsigned int) src/./hb-buffer-private.hh:206:56
    #12 0x4ed70a in hb_buffer_t::make_room_for(unsigned int, unsigned int) src/hb-buffer.cc:134
    #13 0x4ed70a in hb_buffer_t::output_glyph(unsigned int) src/hb-buffer.cc:342
    #14 0x653ba4 in OT::Sequence::apply(OT::hb_apply_context_t*) const src/./hb-ot-layout-gsub-table.hh:291:7
    #15 0x6524c8 in OT::MultipleSubstFormat1::apply(OT::hb_apply_context_t*) const src/./hb-ot-layout-gsub-table.hh:360:5
    #16 0x6524c8 in bool OT::hb_apply_context_t::dispatch<OT::MultipleSubstFormat1>(OT::MultipleSubstFormat1 const&) src/./hb-ot-layout-gsubgpos-private.hh:446
    #17 0x6524c8 in OT::hb_apply_context_t::return_t OT::MultipleSubst::dispatch<OT::hb_apply_context_t>(OT::hb_apply_context_t*) const src/./hb-ot-layout-gsub-table.hh:423
    #18 0x6524c8 in OT::hb_apply_context_t::return_t OT::SubstLookupSubTable::dispatch<OT::hb_apply_context_t>(OT::hb_apply_context_t*, unsigned int) const src/./hb-ot-layout-gsub-table.hh:1080
    #19 0x5ecc79 in OT::hb_apply_context_t::return_t OT::Lookup::dispatch<OT::SubstLookupSubTable, OT::hb_apply_context_t>(OT::hb_apply_context_t*) const src/./hb-ot-layout-common-private.hh:625:»
    #20 0x5ecc79 in OT::hb_apply_context_t::return_t OT::SubstLookup::dispatch<OT::hb_apply_context_t>(OT::hb_apply_context_t*) const src/./hb-ot-layout-gsub-table.hh:1234
    #21 0x5ecc79 in OT::SubstLookup::apply(OT::hb_apply_context_t*) const src/./hb-ot-layout-gsub-table.hh:1127
    #22 0x5ecc79 in _ZL13apply_forwardIN2OT11SubstLookupEEbPNS0_18hb_apply_context_tERKT_RK33hb_ot_layout_lookup_accelerator_t src/hb-ot-layout.cc:898
    #23 0x5ecc79 in _ZL12apply_stringI9GSUBProxyEvPN2OT18hb_apply_context_tERKNT_6LookupERK33hb_ot_layout_lookup_accelerator_t src/hb-ot-layout.cc:976
    #24 0x5fcb93 in void hb_ot_map_t::apply<GSUBProxy>(GSUBProxy const&, hb_ot_shape_plan_t const*, hb_font_t*, hb_buffer_t*) const src/hb-ot-layout.cc:1027:7
    #25 0x5ebe43 in hb_ot_map_t::substitute(hb_ot_shape_plan_t const*, hb_font_t*, hb_buffer_t*) const src/hb-ot-layout.cc:1043:3
    #26 0x53f13d in hb_ot_shape_plan_t::substitute(hb_font_t*, hb_buffer_t*) const src/./hb-ot-shape-private.hh:59:73
    #27 0x53f13d in hb_ot_substitute_complex(hb_ot_shape_context_t*) src/hb-ot-shape.cc:588
    #28 0x53f13d in hb_ot_substitute(hb_ot_shape_context_t*) src/hb-ot-shape.cc:602
    #29 0x53f13d in hb_ot_shape_internal(hb_ot_shape_context_t*) src/hb-ot-shape.cc:818
    #30 0x53f13d in _hb_ot_shape src/hb-ot-shape.cc:839
    #31 0x522686 in hb_shape_plan_execute src/./hb-shaper-list.hh:43:1
    #32 0x51f986 in hb_shape_full src/hb-shape.cc:375:19
    #33 0x51f986 in hb_shape src/hb-shape.cc:405

[behdad]

Yeah, exponential growth in buffer is well-known. Can happen in much shorter fonts even. See eg:
https://github.com/behdad/harfbuzz-hazmat

We have to put an artificial limit in. Something like 16x the starting length...

@jfkthame WDYT? Is there any need for that number to be configurable?

[jfkthame]

I doubt it needs to be configurable, but you might as well use a #define that could be overridden if someone really wants to (or just for testing purposes)....

#ifndef HB_BUFFER_MAX_EXPANSION_FACTOR
#define HB_BUFFER_MAX_EXPANSION_FACTOR 32
#endif

(Picking 32 as default on the grounds that compatibility decomps in Unicode can stretch to at least 18 characters -- U+FDFA is the longest one I can think of offhand. It seems unlikely anyone would really want to do that in a 'ccmp' feature or w/e, but we can claim that it'd at least be possible.)

[behdad]

Good point. 32 it is then. I'll try to implement this soon.

[behdad]

I'm going to make the rule be inputlen * HB_BUFFER_MAX_EXPANSION_FACTOR + 8192 just to make it impossible to hit the condition for real-world use-cases, while still catching abusive ones.

[behdad]

After limiting memory expansion, I also fixed a hang...

I added more -D stuff to hb/test/fuzzing/Makefile.am, specifically:

+       -DHB_BUFFER_MAX_EXPANSION_FACTOR=3 \
+       -DHB_BUFFER_MAX_LEN_MIN=8 \

[kcc]

Now the fuzzer crashes like this (see #157 (comment)):

harfbuzz_asan_cov_fuzzer: hb-buffer.cc:398: bool hb_buffer_t::move_to(unsigned int): Assertion `i <= out_len + (len - idx)' failed.

[behdad]

Thanks. We never tested the OOM codepaths. We are now. Guess I'll postpone a release until next week so I can sort these out.

[behdad]

Now the fuzzer crashes like this (see #157 (comment)):

harfbuzz_asan_cov_fuzzer: hb-buffer.cc:398: bool hb_buffer_t::move_to(unsigned int): Assertion `i <= out_len + (len - idx)' failed.

Should be fixed.

[kcc]

Confirmed, fuzzing restarted, it's running at nice 4000 exec/s

[behdad]

Excellent! I finally go ahead and make that overdue release!