From 0cfe4615c7a3fb7b55295cad09ebe4262e276971 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sat, 31 Oct 2020 09:28:24 -0700 Subject: [PATCH] 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. --- CHANGELOG.md | 1 + plugin/gd/gvrender_gd_vrml.c | 10 ++++++++-- rtest/test_regression.py | 20 ++++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 471265773..039283420 100644 --- 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 0bde14e66..aed01dd6f 100644 --- a/plugin/gd/gvrender_gd_vrml.c +++ b/plugin/gd/gvrender_gd_vrml.c @@ -33,6 +33,7 @@ /* for late_double() */ #include +#include #include /* 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 c57a3d74e..1a496ce94 100644 --- 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 -- 2.40.0