<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 26, 2018 at 12:06 AM, Richard Wordingham <span dir="ltr"><<a href="mailto:richard.wordingham@ntlworld.com" target="_blank">richard.wordingham@ntlworld.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, 24 Jul 2018 16:31:50 +0000 (UTC)<br> <a href="mailto:behdad@kemper.freedesktop.org">behdad@kemper.freedesktop.org</a> (Behdad Esfahbod) wrote:<br> <br> The following change bothers me:<br> <br> > src/hb-ot-layout-common-<wbr>private.hh | 7 +++++++<br> > src/hb-ot-layout.cc | 5 ++++-<br> > 2 files changed, 11 insertions(+), 1 deletion(-)<br> > <br> > New commits:<br> > commit 85646fdadb2f102333485e07425361<wbr>795b4e0412<br> > Author: Garret Rieger <<a href="mailto:grieger@google.com">grieger@google.com</a>><br> > Date: Mon Jul 23 15:37:18 2018 -0700<br> > <br> > [subset] Limit the iterations of the closure algorithm.<br> > Prevents O(n^2) run times.<br> > <br> > diff --git a/src/hb-ot-layout-common-<wbr>private.hh<br> > b/src/hb-ot-layout-common-<wbr>private.hh index 21caf9e9..7ff0dbeb 100644<br> > --- a/src/hb-ot-layout-common-<wbr>private.hh<br> > +++ b/src/hb-ot-layout-common-<wbr>private.hh<br> > @@ -41,6 +41,13 @@<br> > #ifndef HB_MAX_CONTEXT_LENGTH<br> > #define HB_MAX_CONTEXT_LENGTH 64<br> > #endif<br> > +#ifndef HB_CLOSURE_MAX_STAGES<br> > +/*<br> > + * The maximum number of times a lookup can be applied during<br> > shaping.<br> > + * Used to limit the number of iterations of the closure algorithm.<br> > + */<br> > +#define HB_CLOSURE_MAX_STAGES 8<br> > +#endif<br> <br> I presume that this is intended to prevent a denial of service attack,<br></blockquote><div><br></div><div>Correct.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> at the cost of trashing a subset font.<br></blockquote><div><br></div><div>Not really.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> In non-malicious use, how is the victim supposed to detect that and<br> then how he needs to change HarfBuzz or his font? Does he have to read<br> all the text using the subset font simply to detect a problem? How<br> does one test that a font does not hit this limit?</blockquote><div><br></div><div>It's impossible to hit that limit... Ok, it would be impossible if we increase it to 32. I'll do that.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Does one have to<br> iterate over the power set of the supported characters for each<br> script? That's O(2^n) - impossible to do! <br> <br> The description of HB_CLOSURE_MAX_STAGES is completely wrong. I was<br> initially alarmed because I have lookups that are invoked in more than<br> 8 places in substitution subtables. A more accurate, but still not<br> perfect, definition, would be 'the maximum number of times lookup can<br> change a bit of text'.<br></blockquote><div><br></div><div>Nope. Stage is a technical term in HarfBuzz GSUB processing.<br><br></div><div>According to OpenType spec, lookups are processed in increasing order of their indices. This implies that each lookup is processed one. But then the script shaping specs say some features are applied separately. Each of those separated list of features/lookups applied are called one stage. The total number of stages in any shaper is the total number of times a lookup can be applied in theory. Note that this does NOT limit recursion through Context and ChainContext lookups.<br></div><div><br>commit 66ccd8ac405c9c25b37de9eb467a7382880dda35 (HEAD -> master, origin/master)<br>Author: Behdad Esfahbod <<a href="mailto:behdad@behdad.org">behdad@behdad.org</a>><br>Date: Mon Jul 30 17:03:06 2018 -0700<br><br> [serialize] Increase stage count from 8 to 32<br> <br> Indic shaper uses many stages. Now we are provably not limiting<br> functionality whereas the previous limit of 8 was assuming real-world<br> practices.<br><br>diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh<br>index dec237c8..1cf530ea 100644<br>--- a/src/hb-ot-layout-common-private.hh<br>+++ b/src/hb-ot-layout-common-private.hh<br>@@ -45,8 +45,10 @@<br> /*<br> * The maximum number of times a lookup can be applied during shaping.<br> * Used to limit the number of iterations of the closure algorithm.<br>+ * This must be larger than the number of times add_pause() is<br>+ * called in a collect_features call of any shaper.<br> */<br>-#define HB_CLOSURE_MAX_STAGES 8<br>+#define HB_CLOSURE_MAX_STAGES 32<br> #endif<br> <br> <br><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> A limit of 8 does not strike me as obviously generous. Some contextual<br> changes can ripple through a string, and I would not be totally<br> surprised to find that 8+1 or more lookups act on some irreducible<br> strings in my Da Lekh font. The consolations are that there are<br> probably shorter paths to create the resultant glyphs from the input<br> set, and one iteration will often process several lookups in the<br> correct sequence.<br> <br> Richard.<br clear="all"></blockquote></div><br>-- <br><div class="gmail_signature">behdad<br><a href="http://behdad.org/" target="_blank">http://behdad.org/</a></div> </div></div>