Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
> I'm not sure what exactly points of discussion are, but in HarfBuzz > we do purposefully ignore position overflows. We use a macro like > this for that: > > #define HB_NO_SANITIZE_SIGNED_INTEGER_OVERFLOW > __attribute__((no_sanitize("signed-integer-overflow"))) > > If you want to copy the macro, please copy its surrounding > conditions as well. For my taste, it is too imprecise, since the attribute can only be set function-wise. You can find the discussion somewhere in freetype-devel, IIRC. Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
> @Werner: should I apply it? Yes, please. Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
I'm not sure what exactly points of discussion are, but in HarfBuzz we do purposefully ignore position overflows. We use a macro like this for that: #define HB_NO_SANITIZE_SIGNED_INTEGER_OVERFLOW __attribute__((no_sanitize("signed-integer-overflow"))) If you want to copy the macro, please copy its surrounding conditions as well. b On Tue, Aug 13, 2019 at 3:04 PM wrote: > >> Whether with -wrapv or with the unsigned macros, we simply disable > >> some compiler optimizations, perhaps some good optimizations too. > > > > Most certainly, yes. But there are a lot more things that slow down the > > potential performance of FreeType -- C in itself is a trade-off between > > maintainability and performance. > > > >> Why? Is it because we got scared? There is absolutely no evidence of > >> real bugs in FreeType. It is reasonable to disable optimizations with > >> -wrapv, if scared, but macros are too rigid. Some compilers recognize > >> /* fall through */ comment to suppress particular warnings. I wish we > >> could just add a comment to silence these warnings after adjudication. > > > > It's not about being scared but about making sure we understand _what_ > the > > code within FT does. If we proactively mark operations that have a > certain > > behaviour we make those operations explicit to whomever reads that thing > in > > the future. `-wrapv' has two downsides IMO: (1) we lose track of what > it > > does and _where_ ... maybe, really covering up bugs (2) we rely on 3rd > parties > > to compile FT in a very specific way to avoid certain types of reports. > > For reference, find the patch attached (most code in that area is already > wrapped in `*_LONG' macros). > > @Werner: should I apply it? > > Armin > ___ > Freetype-devel mailing list > Freetype-devel@nongnu.org > https://lists.nongnu.org/mailman/listinfo/freetype-devel > -- behdad http://behdad.org/ ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
>> Whether with -wrapv or with the unsigned macros, we simply disable >> some compiler optimizations, perhaps some good optimizations too. > > Most certainly, yes. But there are a lot more things that slow down the > potential performance of FreeType -- C in itself is a trade-off between > maintainability and performance. > >> Why? Is it because we got scared? There is absolutely no evidence of >> real bugs in FreeType. It is reasonable to disable optimizations with >> -wrapv, if scared, but macros are too rigid. Some compilers recognize >> /* fall through */ comment to suppress particular warnings. I wish we >> could just add a comment to silence these warnings after adjudication. > > It's not about being scared but about making sure we understand _what_ the > code within FT does. If we proactively mark operations that have a certain > behaviour we make those operations explicit to whomever reads that thing in > the future. `-wrapv' has two downsides IMO: (1) we lose track of what it > does and _where_ ... maybe, really covering up bugs (2) we rely on 3rd parties > to compile FT in a very specific way to avoid certain types of reports. For reference, find the patch attached (most code in that area is already wrapped in `*_LONG' macros). @Werner: should I apply it? Armin 977845.patch Description: Binary data ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
> Whether with -wrapv or with the unsigned macros, we simply disable some > compiler optimizations, perhaps some good optimizations too. Most certainly, yes. But there are a lot more things that slow down the potential performance of FreeType -- C in itself is a trade-off between maintainability and performance. > Why? Is it because we got scared? There is absolutely no evidence of real bugs > in FreeType. It is reasonable to disable optimizations with -wrapv, if > scared, but macros are too rigid. Some compilers recognize /* fall through */ > comment to suppress particular warnings. I wish we could just add a comment to > silence these warnings after adjudication. It's not about being scared but about making sure we understand _what_ the code within FT does. If we proactively mark operations that have a certain behaviour we make those operations explicit to whomever reads that thing in the future. `-wrapv' has two downsides IMO: (1) we lose track of what it does and _where_ ... maybe, really covering up bugs (2) we rely on 3rd parties to compile FT in a very specific way to avoid certain types of reports. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
On Sat, Aug 10, 2019 at 7:32 PM Nikolaus Waxweiler wrote: > > > Undefined does not mean scary. > > Actually yes. Have you read e.g. > http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html? > Whether with -wrapv or with the unsigned macros, we simply disable some compiler optimizations, perhaps some good optimizations too. Why? Is it because we got scared? There is absolutely no evidence of real bugs in FreeType. It is reasonable to disable optimizations with -wrapv, if scared, but macros are too rigid. Some compilers recognize /* fall through */ comment to suppress particular warnings. I wish we could just add a comment to silence these warnings after adjudication. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
Undefined does not mean scary. Actually yes. Have you read e.g. http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html? Why do we even care? The burden is actually on the compiler to not do anything crazy or face consequences from users and public. For some reason the burden is put on the innocent and quite reasonable public. This does not make sense. I think if you are innocent and reasonable, you don't use C/C++ 😁 ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
> This, sadly, brings us back to the current way of dealing with these things; > adding ugly macros that transfer these operations from UB space into defined > C space ... Not saying I'm happy with that but I believe this is the > cleanest solution in the big picture right now. Undefined does not mean prohibited. Undefined does not mean scary. Undefined does not mean immoral. Why do we even care? The burden is actually on the compiler to not do anything crazy or face consequences from users and public. For some reason the burden is put on the innocent and quite reasonable public. This does not make sense. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
>> .. and undo those macros? > > Well, if you then can? Signed integer overflow being undefined strikes me as a > severe deficiency in the C language. This of course makes -wrapv a compiler > level workaround, which may not be available to every compiler FreeType wants > to support. Hm. It's one of many oddities of C ... Problem is that it's undefined behaviour and compilers are technically free to do anything with the calculation, including optimising statements away completely that have to be preserved when assuming wrap-around that could, maybe, end up becoming an actual bug with buffer overflow or whatever. Haven't constructed such an example yet though. Also, I agree with both of you: I think the current solution (casting everything for the sole purpose of silencing UB fuzzers) is an ugly way or introducing technically irrelevant overhead to the code base. On the other hand; the C standard is what the C standard is (maybe for good reason, maybe due to lobbying) but at least it is a standard that is being obeyed. As such, I don't think it's a good idea to suggest people should build FreeType in ways that change the C standard on a compiler level. This, sadly, brings us back to the current way of dealing with these things; adding ugly macros that transfer these operations from UB space into defined C space ... Not saying I'm happy with that but I believe this is the cleanest solution in the big picture right now. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
> > .. and undo those macros? > Well, if you then can? Signed integer overflow being undefined strikes me as a severe deficiency in the C language. This of course makes -wrapv a compiler level workaround, which may not be available to every compiler FreeType wants to support. Hm. > ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
On Fri, Aug 9, 2019 at 2:38 PM Nikolaus Waxweiler wrote: > > This makes me wonder if maybe FreeType should be compiled with -wrapv > by default? .. and undo those macros? These warnings are OCD in its ugliest. It is fleetingly rare when they reveal real bugs. We silence them but pay dearly with code readability. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
This makes me wonder if maybe FreeType should be compiled with -wrapv by default? ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
> Thanks for looking into it. FWIW, my commit merely re-enabled an older code > path. NW we've already fixed a lot of those that came up in our own fuzzers :) ... it's actually harmless but a side product of fuzzers throwing super random input at apps. Some overflow when facing gigantic glyphs but no one who's truly interested in rendered results would ever use FreeType with such inputs. The idea is to keep the overflow as it is, but have it done in `unsigned' world where it is actually "defined" behaviour (resulting in the same value most likely tho). I left a comment at https://bugs.chromium.org/p/chromium/issues/detail?id=977845#c7 which would help me figure out the exact line of peril much more quickly :) Armin ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
Thanks for looking into it. FWIW, my commit merely re-enabled an older code path. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
Thanks for sending that in -- looks harmless but is worth fixing; if for nothing else but avoiding these kinds of reports in apps that fuzz FreeType ;) I'll happily look into it later and provide a fix. Armin -Original Message- From: Freetype-devel On Behalf Of Nikolaus Waxweiler Sent: 06 August 2019 20:08 To: freetype-devel Subject: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics Forwarding the following message I received regarding a fuzzer find. I'm not sure what to do about it. -- Weitergeleitete Nachricht -- Von: kkal… via monorail Betreff: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics Datum: Wed, 10 Jul 2019 00:34:24 -0700 An: madig...@gmail.com { "@context": "http://schema.org";, "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "name": "View Issue", "url": "https://bugs.chromium.org/p/chromium/issues/detail?id=977845"; }, "description": "" } Updates: Labels: M-76 Test-Predator-Wrong Comment #5 on issue 977845 by kkal...@chromium.org: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics https://bugs.chromium.org/p/chromium/issues/detail?id=977845#c5 Unable to provide possible suspect using Predator, CL and Code Search. Could someone please look into the issue. Thank You... -- You received this message because: 1. You were specifically CC'd on the issue You may adjust your notification preferences at: https://bugs.chromium.org/hosting/settings Reply to this email to add a comment. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
[ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics
Forwarding the following message I received regarding a fuzzer find. I'm not sure what to do about it. -- Weitergeleitete Nachricht -- Von: kkal… via monorail Betreff: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics Datum: Wed, 10 Jul 2019 00:34:24 -0700 An: madig...@gmail.com { "@context": "http://schema.org";, "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "name": "View Issue", "url": "https://bugs.chromium.org/p/chromium/issues/detail?id=977845"; }, "description": "" } Updates: Labels: M-76 Test-Predator-Wrong Comment #5 on issue 977845 by kkal...@chromium.org: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics https://bugs.chromium.org/p/chromium/issues/detail?id=977845#c5 Unable to provide possible suspect using Predator, CL and Code Search. Could someone please look into the issue. Thank You... -- You received this message because: 1. You were specifically CC'd on the issue You may adjust your notification preferences at: https://bugs.chromium.org/hosting/settings Reply to this email to add a comment. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel