GraphicsMagick: Remaining fixes based on Coverity analysis.
GraphicsMagick Commits <[email protected]>
| Newsgroups | gmane.comp.video.graphicsmagick.cvs |
|---|---|
| Message-ID | <mailman.53596.1648843775.1623.graphicsmagick-commit@lists.sourceforge.net> |
changeset 3b9a5a79c0d7 in /hg/GraphicsMagick details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=3b9a5a79c0d7 summary: Remaining fixes based on Coverity analysis. diffstat: ChangeLog | 14 ++++++++++++++ coders/msl.c | 17 +++++++++++++++-- coders/svg.c | 18 ++++++++++++++++-- magick/display.c | 9 ++++++--- www/Changelog.html | 10 ++++++++++ www/index.html | 5 +++-- www/index.rst | 6 ++++-- 7 files changed, 68 insertions(+), 11 deletions(-) diffs (165 lines): diff -r 8c5fdda5297d -r 3b9a5a79c0d7 ChangeLog --- a/ChangeLog Fri Apr 01 09:04:54 2022 -0500 +++ b/ChangeLog Fri Apr 01 15:09:22 2022 -0500 @@ -1,5 +1,19 @@ 2022-04-01 Bob Friesenhahn <[email protected]> + * www/index.rst: Update the Coverity Analysis Metrics. + + * magick/display.c (MagickXAnnotateEditImage): Quiet Coverity + 376901 "Identical code for different branches + (IDENTICAL_BRANCHES)". + + * coders/svg.c (ReadSVGImage): Default to not allowing external + entity substitution. Quiets Coverity 376905 + "unsafe_xml_parse_config (UNSAFE_XML_PARSE_CONFIG)". + + * coders/msl.c (ProcessMSLScript): Default to not allowing + external entity substitution. Quiets Coverity 376913 + "unsafe_xml_parse_config (UNSAFE_XML_PARSE_CONFIG)". + * magick/error.c (ThrowLoggedException): Silence Coverity 376912 "Dereference after null check (FORWARD_NULL)". diff -r 8c5fdda5297d -r 3b9a5a79c0d7 coders/msl.c --- a/coders/msl.c Fri Apr 01 09:04:54 2022 -0500 +++ b/coders/msl.c Fri Apr 01 15:09:22 2022 -0500 @@ -1,5 +1,5 @@ /* -% Copyright (C) 2003 - 2021 GraphicsMagick Group +% Copyright (C) 2003 - 2022 GraphicsMagick Group % Copyright (C) 2002 ImageMagick Studio % % This program is covered by multiple licenses, which are described in @@ -4572,7 +4572,20 @@ msl_info.image[0]=msl_image; if (writer_image != (Image *) NULL) MSLPushImage(&msl_info,writer_image); - (void) xmlSubstituteEntitiesDefault(1); + /* + xmlSubstituteEntitiesDefault(1) enables external ENTITY support + (e.g. SVGResolveEntity() which allows XML to be downloaded from an + external source. This may be a security hazard if the input is + not trustworty or if connecting to the correct source is not + assured. If the XML is parsed on the backside of a firewall then + it may be able to access unintended resources. + + See "https://hdivsecurity.com/owasp-xml-external-entities-xxe". + + FIXME: Do we need a way for the user to enable this? Does + retrieval of external entities work at all? + */ + (void) xmlSubstituteEntitiesDefault(0); (void) memset(&SAXModules,0,sizeof(SAXModules)); SAXModules.internalSubset=MSLInternalSubset; diff -r 8c5fdda5297d -r 3b9a5a79c0d7 coders/svg.c --- a/coders/svg.c Fri Apr 01 09:04:54 2022 -0500 +++ b/coders/svg.c Fri Apr 01 15:09:22 2022 -0500 @@ -1,5 +1,5 @@ /* -% Copyright (C) 2003-2021 GraphicsMagick Group +% Copyright (C) 2003-2022 GraphicsMagick Group % Copyright (C) 2002 ImageMagick Studio % % This program is covered by multiple licenses, which are described in @@ -4058,7 +4058,21 @@ if (image_info->size != (char *) NULL) (void) CloneString(&svg_info.size,image_info->size); (void) LogMagickEvent(CoderEvent,GetMagickModule(),"begin SAX"); - (void) xmlSubstituteEntitiesDefault(1); + /* + xmlSubstituteEntitiesDefault(1) enables external ENTITY support + (e.g. SVGResolveEntity() which allows XML to be downloaded from an + external source. This may be a security hazard if the input is + not trustworty or if connecting to the correct source is not + assured. If the XML is parsed on the backside of a firewall then + it may be able to access unintended resources. + + See "https://www.w3.org/TR/SVG11/svgdtd.html#DTD.1.16" and + "https://hdivsecurity.com/owasp-xml-external-entities-xxe". + + FIXME: Do we need a way for the user to enable this? Does + retrieval of external entities work at all? + */ + (void) xmlSubstituteEntitiesDefault(0); (void) memset(&SAXModules,0,sizeof(SAXModules)); SAXModules.internalSubset=SVGInternalSubset; diff -r 8c5fdda5297d -r 3b9a5a79c0d7 magick/display.c --- a/magick/display.c Fri Apr 01 09:04:54 2022 -0500 +++ b/magick/display.c Fri Apr 01 15:09:22 2022 -0500 @@ -2246,9 +2246,12 @@ annotate_context,x,y,p,1); x+=XTextWidth(font_info,p,1); p++; - if ((x+font_info->max_bounds.width) < (int) windows->image.width) - break; - break; /* Not completely sure about this break (used to fall through) */ + /* + This was active: + if ((x+font_info->max_bounds.width) < (int) windows->image.width) + break; + */ + break; /* FIXME: Not completely sure about this break (used to fall through) */ } case XK_Return: case XK_KP_Enter: diff -r 8c5fdda5297d -r 3b9a5a79c0d7 www/Changelog.html --- a/www/Changelog.html Fri Apr 01 09:04:54 2022 -0500 +++ b/www/Changelog.html Fri Apr 01 15:09:22 2022 -0500 @@ -37,6 +37,16 @@ <p>2022-04-01 Bob Friesenhahn <<a class="reference external" href="mailto:bfriesen%40simple.dallas.tx.us">bfriesen<span>@</span>simple<span>.</span>dallas<span>.</span>tx<span>.</span>us</a>></p> <blockquote> +<p>* www/index.rst: Update the Coverity Analysis Metrics.</p> +<p>* magick/display.c (MagickXAnnotateEditImage): Quiet Coverity +376901 "Identical code for different branches +(IDENTICAL_BRANCHES)".</p> +<p>* coders/svg.c (ReadSVGImage): Default to not allowing external +entity substitution. Quiets Coverity 376905 +"unsafe_xml_parse_config (UNSAFE_XML_PARSE_CONFIG)".</p> +<p>* coders/msl.c (ProcessMSLScript): Default to not allowing +external entity substitution. Quiets Coverity 376913 +"unsafe_xml_parse_config (UNSAFE_XML_PARSE_CONFIG)".</p> <p>* magick/error.c (ThrowLoggedException): Silence Coverity 376912 "Dereference after null check (FORWARD_NULL)".</p> <p>* coders/jp2.c (ReadJP2Image): Silence Coverity 264883 "Division diff -r 8c5fdda5297d -r 3b9a5a79c0d7 www/index.html --- a/www/index.html Fri Apr 01 09:04:54 2022 -0500 +++ b/www/index.html Fri Apr 01 15:09:22 2022 -0500 @@ -121,8 +121,9 @@ <li>GM source code is managed in <a class="reference external" href="https://www.mercurial-scm.org/">Mercurial</a>, a distributed source control management tool which supports management of local changes. The repository history goes back to 1998.</li> -<li>GM has 0.00 (zero) defects per 1000 lines of code (293,341 total -lines included) according to Coverity analysis on May 25, 2015.</li> +<li>GM has 0.00 (zero) defects per 1000 lines of code (332,286 total +lines included) according to <a class="reference external" href="https://scan.coverity.com/projects/graphicsmagick">Coverity Analysis Metrics</a> on April +1, 2022.</li> <li>GM developers contribute to other free projects for the public good.</li> </ul> </blockquote> diff -r 8c5fdda5297d -r 3b9a5a79c0d7 www/index.rst --- a/www/index.rst Fri Apr 01 09:04:54 2022 -0500 +++ b/www/index.rst Fri Apr 01 15:09:22 2022 -0500 @@ -64,6 +64,7 @@ .. _`ASan` : https://github.com/google/sanitizers/wiki/AddressSanitizer .. _`UBSan` : https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html .. _`oss-fuzz` : https://github.com/google/oss-fuzz +.. _`Coverity Analysis Metrics` : https://scan.coverity.com/projects/graphicsmagick GraphicsMagick is the swiss army knife of image processing. Comprised of 279K physical lines (according to David A. Wheeler's `SLOCCount`_) @@ -145,8 +146,9 @@ control management tool which supports management of local changes. The repository history goes back to 1998. - * GM has 0.00 (zero) defects per 1000 lines of code (293,341 total - lines included) according to Coverity analysis on May 25, 2015. + * GM has 0.00 (zero) defects per 1000 lines of code (332,286 total + lines included) according to `Coverity Analysis Metrics`_ on April + 1, 2022. * GM developers contribute to other free projects for the public good.