fix: fclose(NULL) in the VRML plugin when current directory is not writable

The VRML plugin writes PNG files of each node in the graph alongside the VRML
output that then references these files. If you give no output location (do not
pass `-o` to dot), the VRML output is written to stdout and the node PNG files
are written to the current directory. However, the plugin was not checking
whether opening of these PNG files succeeded. As a result, if the current
directory was not writable, it would end up calling fclose() with a NULL
pointer.

This change makes the plugin report the failure to open PNG files and cause dot
to eventually exit with non-zero. Note that VRML processing is not stopped, so
the user can still get complete VRML output, albeit missing the associated PNG
files that will be referenced.

The test case included in this commit does not necessarily reproduce the failure
prior to these changes. This is because Glibc allows fclose(NULL). However, this
is beyond the spec and other libcs do not provide these guarantees.

Fixes #793.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4712657735282fb908a5b9a0d783629d9c839343..0392834204df91b3ec6588d51e30469a6cb30b3c 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -62,6 +62,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - multiple graphs to file output causes a segfault #1845
 - lefty PTY functionality relies on file descriptor implementation details #1823
 - buffer overflow in fdpgen
+- Crashes by VRML output when current directory is not writable #793
 
 ## [2.44.1] - 2020-06-29
 

diff --git a/plugin/gd/gvrender_gd_vrml.c b/plugin/gd/gvrender_gd_vrml.c
index 0bde14e66a70616f31ba268d53aef883bb262846..aed01dd6f0970156ddc136d9d795231620ef3390 100644 (file)
--- a/plugin/gd/gvrender_gd_vrml.c
+++ b/plugin/gd/gvrender_gd_vrml.c
@@ -33,6 +33,7 @@
 
 /* for late_double() */
 #include <cgraph/agxbuf.h>
+#include <cgraph/cgraph.h>
 #include <common/utils.h>
 
 /* for wind() */
@@ -242,6 +243,9 @@ static void vrml_begin_node(GVJ_t *job)
        MinZ = z;
     if (shapeOf(n) != SH_POINT) {
        PNGfile = nodefile(job->output_filename, n);
+       if (PNGfile == NULL) {
+               agerrorf("failed to open file for writing PNG node image\n");
+       }
 
        width  = (ND_lw(n) + ND_rw(n)) * Scale + 2 * NODE_PAD;
        height = (ND_ht(n)           ) * Scale + 2 * NODE_PAD;
@@ -258,8 +262,10 @@ static void vrml_begin_node(GVJ_t *job)
 static void vrml_end_node(GVJ_t *job)
 {
     if (im) {
-       gdImagePng(im, PNGfile);
-       fclose(PNGfile);
+       if (PNGfile != NULL) {
+               gdImagePng(im, PNGfile);
+               fclose(PNGfile);
+       }
        gdImageDestroy(im);
        im = NULL;
     }

diff --git a/rtest/test_regression.py b/rtest/test_regression.py
index c57a3d74e775650e019c39897a21538bdcdecebf..1a496ce94805421efa6ba130d56ad6ed0c7cc2de 100644 (file)
--- a/rtest/test_regression.py
+++ b/rtest/test_regression.py
@@ -5,6 +5,7 @@ import signal
 import subprocess
 import os
 import re
+import stat
 import tempfile
 
 # The terminology used in rtest.py is a little inconsistent. At the
@@ -102,6 +103,25 @@ def test_165_3():
     assert any(r'hello \\\" world' in l for l in ldraw), \
       'unexpected ldraw contents'
 
+def test_793():
+    '''
+    Graphviz should not crash when using VRML output with a non-writable current
+    directory
+    https://gitlab.com/graphviz/graphviz/-/issues/793
+    '''
+
+    # create a non-writable directory
+    with tempfile.TemporaryDirectory() as tmp:
+      os.chmod(tmp, os.stat(tmp).st_mode & ~stat.S_IWRITE)
+
+      # ask the VRML back end to handle a simple graph, using the above as the
+      # current working directory
+      p = subprocess.Popen(['dot', '-Tvrml', '-o', os.devnull], cwd=tmp)
+      p.communicate('digraph { a -> b; }')
+
+    # Graphviz should not have caused a segfault
+    assert p.returncode != -signal.SIGSEGV, 'Graphviz segfaulted'
+
 def test_1314():
     '''
     test that a large font size that produces an overflow in Pango is rejected