From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Sun, 1 Nov 2020 02:41:27 +0000 (-0700)
Subject: fix buffer under-read in fdp
X-Git-Tag: 2.46.0~17^2~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8de8c4795fb556bff9801856c92dc94c4d48ecbc;p=graphviz

fix buffer under-read in fdp

When using fdp to process a graph, it would try to canonicalize all node names,
regardless of whether they were hosted in refstr_ts or not. The problem with
this is that the canonicalization logic assumes the character pointer passed
into it is within a refstr_t and that therefore it can do pointer subtraction to
get a pointer to the refstr_t itself. This was not true for internally
constructed node names like "%4".

We change this to always go through the refstr logic in agstrdup() when
canonicalizing names. This ensures that the pointer we pass into the
canonicalization logic *is* within a refstr_t. This is based on canon() in
lib/cgraph/output.c. Fixes #236, #1290, #1865.
---

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 015ee5afd..ca70fc6e9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -64,6 +64,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - buffer overflow in fdpgen
 - Crashes by VRML output when current directory is not writable #793
 - Segmentation fault when newrank=true #1221
+- sfdp craches #236
+- fdp segmentation fault with GK=0 #1290
+- fdp crash #1865
 
 ## [2.44.1] - 2020-06-29
 
diff --git a/lib/cgraph/write.c b/lib/cgraph/write.c
index dbfb57d4b..af920fab7 100644
--- a/lib/cgraph/write.c
+++ b/lib/cgraph/write.c
@@ -217,7 +217,18 @@ static int _write_canonstr(Agraph_t * g, iochan_t * ofile, char *str,
 
 static int write_canonstr(Agraph_t * g, iochan_t * ofile, char *str)
 {
-    return _write_canonstr(g, ofile, str, TRUE);
+    char *s;
+    int r;
+
+    /* str may not have been allocated by agstrdup, so we first need to turn it
+     * into a valid refstr
+     */
+    s = agstrdup(g, str);
+
+    r = _write_canonstr(g, ofile, s, TRUE);
+
+    agstrfree(g, s);
+    return r;
 }
 
 static int write_dict(Agraph_t * g, iochan_t * ofile, char *name,
diff --git a/rtest/1865.dot b/rtest/1865.dot
new file mode 100644
index 000000000..83e2b00e8
--- /dev/null
+++ b/rtest/1865.dot
@@ -0,0 +1,6 @@
+strict digraph "FTC/BTC" {
+subgraph cluster_Inputs {
+"Base (FTC) holding"
+}
+}
+
diff --git a/rtest/test_regression.py b/rtest/test_regression.py
index f43312e86..1daf11944 100644
--- a/rtest/test_regression.py
+++ b/rtest/test_regression.py
@@ -401,6 +401,22 @@ def test_1845():
     # generate a multipage PS file from this input
     subprocess.check_call(['dot', '-Tps', '-o', os.devnull, input])
 
+@pytest.mark.skipif(shutil.which('fdp') is None, reason='fdp not available')
+def test_1865():
+    '''
+    fdp should not read out of bounds when processing node names
+    https://gitlab.com/graphviz/graphviz/-/issues/1865
+    Note, the crash this test tries to provoke may only occur when run under
+    Address Sanitizer or Valgrind
+    '''
+
+    # locate our associated test case in this directory
+    input = os.path.join(os.path.dirname(__file__), '1865.dot')
+    assert os.path.exists(input), 'unexpectedly missing test case'
+
+    # fdp should not crash when processing this file
+    subprocess.check_call(['fdp', '-o', os.devnull, input])
+
 # root directory of this checkout
 ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))