[prev in list] [next in list] [prev in thread] [next in thread]
List: graphicsmagick-commit
Subject: [GM-commit] GraphicsMagick: Remaining fixes based on Coverity analysis.
From: GraphicsMagick Commits <graphicsmagick-commit () lists ! sourceforge ! net>
Date: 2022-04-01 20:09:25
Message-ID: mailman.53596.1648843775.1623.graphicsmagick-commit () lists ! sourceforge ! net
[Download RAW message or body]
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 <bfriesen@simple.dallas.tx.us>
+ * 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> \
4;</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.
_______________________________________________
Graphicsmagick-commit mailing list
Graphicsmagick-commit@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/graphicsmagick-commit
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic