Re: [ft-devel] Fwd: Issue 977845 in chromium: pdf_font_fuzzer: Integer-overflow in compute_glyph_metrics

2019-08-14 Thread Werner LEMBERG

> 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

2019-08-14 Thread Werner LEMBERG

> @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

2019-08-13 Thread Behdad Esfahbod
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

2019-08-13 Thread armin
>> 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

2019-08-13 Thread armin
> 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

2019-08-12 Thread Alexei Podtelezhnikov
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

2019-08-10 Thread Nikolaus Waxweiler



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

2019-08-10 Thread Alexei Podtelezhnikov

> 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

2019-08-10 Thread armin
>> .. 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

2019-08-10 Thread Nikolaus Waxweiler
>
> .. 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

2019-08-09 Thread Alexei Podtelezhnikov
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

2019-08-09 Thread Nikolaus Waxweiler
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

2019-08-07 Thread armin
> 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

2019-08-06 Thread Nikolaus Waxweiler
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

2019-08-06 Thread armin
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