From d49732b119b621d34523f4f014b8b4a2bacfb7c6 Mon Sep 17 00:00:00 2001 From: sabith saheb Date: Tue, 20 Oct 2026 16:06:52 +0530 Subject: [PATCH 1/3] internal/graph: escape file and title strings written to dot Mapping/object file names and the graph title come from the profile and are attacker-controlled, but three sites wrote them into the dot output without escaping: the digraph name in start(), the legend tooltip in addLegend(), and the File/Objfile components of a node label in multilinePrintableName(). A double quote in any of them closes the quoted attribute and lets the profile inject further dot attributes (for example a URL=), which Graphviz emits as an xlink:href in the SVG that the web UI renders inline. Wrap each site in escapeForDot, matching the sibling node and edge tooltips that already escape. --- internal/graph/dotgraph.go | 9 ++++++--- internal/graph/dotgraph_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/internal/graph/dotgraph.go b/internal/graph/dotgraph.go index d704fc4e13..72c942940e 100644 --- a/internal/graph/dotgraph.go +++ b/internal/graph/dotgraph.go @@ -110,7 +110,7 @@ func (b *builder) start() { if b.config.Title != "" { graphname = b.config.Title } - fmt.Fprintln(b, `digraph "`+graphname+`" {`) + fmt.Fprintln(b, `digraph "`+escapeForDot(graphname)+`" {`) fmt.Fprintln(b, `node [style=filled fillcolor="#f8f8f8"]`) } @@ -132,7 +132,7 @@ func (b *builder) addLegend() { fmt.Fprintf(b, ` URL="%s" target="_blank"`, b.config.LegendURL) } if b.config.Title != "" { - fmt.Fprintf(b, ` tooltip="%s"`, b.config.Title) + fmt.Fprintf(b, ` tooltip="%s"`, escapeForDot(b.config.Title)) } fmt.Fprintf(b, "] }\n") } @@ -390,7 +390,10 @@ func multilinePrintableName(info *NodeInfo) string { infoCopy.Name = strings.ReplaceAll(infoCopy.Name, "[...]", "[…]") infoCopy.Name = strings.ReplaceAll(infoCopy.Name, ".", `\n`) if infoCopy.File != "" { - infoCopy.File = filepath.Base(infoCopy.File) + infoCopy.File = escapeForDot(filepath.Base(infoCopy.File)) + } + if infoCopy.Objfile != "" { + infoCopy.Objfile = escapeForDot(filepath.Base(infoCopy.Objfile)) } return strings.Join(infoCopy.NameComponents(), `\n`) + `\n` } diff --git a/internal/graph/dotgraph_test.go b/internal/graph/dotgraph_test.go index b375bb6225..ed076682dd 100644 --- a/internal/graph/dotgraph_test.go +++ b/internal/graph/dotgraph_test.go @@ -150,6 +150,33 @@ func TestComposeWithNamesThatNeedEscaping(t *testing.T) { compareGraphs(t, buf.Bytes(), "compose7.dot") } +func TestComposeEscapesFileAndTitleInDot(t *testing.T) { + // A mapping/object file name and the graph title come from the profile and + // are attacker-controlled. A double quote in any of them must be escaped so + // it cannot break out of the quoted dot attribute and inject new attributes + // (e.g. a URL= that Graphviz turns into a javascript: link in the SVG). + g := baseGraph() + a, c := baseAttrsAndConfig() + g.Nodes[0].Info = NodeInfo{Name: "fn", File: `/src/evil.go" peripheries=3`, Lineno: 1} + g.Nodes[1].Info = NodeInfo{Address: 0x10, Objfile: `/lib/evil.so" peripheries=3`} + c.Title = `prof" bgcolor="red` + + var buf bytes.Buffer + ComposeDot(&buf, g, a, c) + out := buf.String() + + for _, raw := range []string{`evil.go" peripheries=3`, `evil.so" peripheries=3`, `prof" bgcolor="red`} { + if strings.Contains(out, raw) { + t.Errorf("unescaped %q in dot output allows attribute injection:\n%s", raw, out) + } + } + for _, escaped := range []string{`evil.go\" peripheries=3`, `evil.so\" peripheries=3`, `prof\" bgcolor=\"red`} { + if !strings.Contains(out, escaped) { + t.Errorf("expected escaped %q in dot output:\n%s", escaped, out) + } + } +} + func TestComposeWithCommentsWithNewlines(t *testing.T) { g := baseGraph() a, c := baseAttrsAndConfig() From 2f477936783fe68f98d3f6256e8e319e1b3246b3 Mon Sep 17 00:00:00 2001 From: sabith saheb Date: Thu, 22 Oct 2026 10:44:05 +0530 Subject: [PATCH 2/3] internal/graph: check dot escaping directly in TestComposeWithNamesThatNeedEscaping Fold the file, objfile and title cases into the existing escaping test and have it assert on the escaped and unescaped forms of each string directly, so the compose7.dot golden is no longer needed. --- internal/graph/dotgraph_test.go | 36 +++++++--------------------- internal/graph/testdata/compose7.dot | 7 ------ 2 files changed, 9 insertions(+), 34 deletions(-) delete mode 100644 internal/graph/testdata/compose7.dot diff --git a/internal/graph/dotgraph_test.go b/internal/graph/dotgraph_test.go index ed076682dd..547266692d 100644 --- a/internal/graph/dotgraph_test.go +++ b/internal/graph/dotgraph_test.go @@ -141,38 +141,20 @@ func TestComposeWithStandardGraphAndURL(t *testing.T) { func TestComposeWithNamesThatNeedEscaping(t *testing.T) { g := baseGraph() a, c := baseAttrsAndConfig() - g.Nodes[0].Info = NodeInfo{Name: `var"src"`} - g.Nodes[1].Info = NodeInfo{Name: `var"#dest#"`} + g.Nodes[0].Info = NodeInfo{Name: `var"src"`, File: `/src/file"name.go`, Lineno: 10} + g.Nodes[1].Info = NodeInfo{Address: 0x10, Objfile: `/lib/obj"file.so`} + c.Title = `test"title` var buf bytes.Buffer ComposeDot(&buf, g, a, c) + got := buf.String() - compareGraphs(t, buf.Bytes(), "compose7.dot") -} - -func TestComposeEscapesFileAndTitleInDot(t *testing.T) { - // A mapping/object file name and the graph title come from the profile and - // are attacker-controlled. A double quote in any of them must be escaped so - // it cannot break out of the quoted dot attribute and inject new attributes - // (e.g. a URL= that Graphviz turns into a javascript: link in the SVG). - g := baseGraph() - a, c := baseAttrsAndConfig() - g.Nodes[0].Info = NodeInfo{Name: "fn", File: `/src/evil.go" peripheries=3`, Lineno: 1} - g.Nodes[1].Info = NodeInfo{Address: 0x10, Objfile: `/lib/evil.so" peripheries=3`} - c.Title = `prof" bgcolor="red` - - var buf bytes.Buffer - ComposeDot(&buf, g, a, c) - out := buf.String() - - for _, raw := range []string{`evil.go" peripheries=3`, `evil.so" peripheries=3`, `prof" bgcolor="red`} { - if strings.Contains(out, raw) { - t.Errorf("unescaped %q in dot output allows attribute injection:\n%s", raw, out) + for _, s := range []string{`var"src"`, `file"name.go`, `obj"file.so`, `test"title`, `label3: "foo"`} { + if strings.Contains(got, s) { + t.Errorf("unescaped %q in dot output:\n%s", s, got) } - } - for _, escaped := range []string{`evil.go\" peripheries=3`, `evil.so\" peripheries=3`, `prof\" bgcolor=\"red`} { - if !strings.Contains(out, escaped) { - t.Errorf("expected escaped %q in dot output:\n%s", escaped, out) + if want := escapeForDot(s); !strings.Contains(got, want) { + t.Errorf("escaped %q not found in dot output:\n%s", want, got) } } } diff --git a/internal/graph/testdata/compose7.dot b/internal/graph/testdata/compose7.dot deleted file mode 100644 index 8f749a7797..0000000000 --- a/internal/graph/testdata/compose7.dot +++ /dev/null @@ -1,7 +0,0 @@ -digraph "testtitle" { -node [style=filled fillcolor="#f8f8f8"] -subgraph cluster_L { "label1" [shape=box fontsize=16 label="label1\llabel2\llabel3: \"foo\"\l" tooltip="testtitle"] } -N1 [label="var\"src\"\n10 (10.00%)\nof 25 (25.00%)" id="node1" fontsize=22 shape=box tooltip="var\"src\" (25)" color="#b23c00" fillcolor="#edddd5"] -N2 [label="var\"#dest#\"\n15 (15.00%)\nof 25 (25.00%)" id="node2" fontsize=24 shape=box tooltip="var\"#dest#\" (25)" color="#b23c00" fillcolor="#edddd5"] -N1 -> N2 [label=" 10" weight=11 color="#b28559" tooltip="var\"src\" -> var\"#dest#\" (10)" labeltooltip="var\"src\" -> var\"#dest#\" (10)"] -} From 6492d0b88088d33c2487468b678511385e13f81b Mon Sep 17 00:00:00 2001 From: sabith saheb Date: Fri, 23 Oct 2026 12:50:48 +0530 Subject: [PATCH 3/3] internal/graph: spell out expected escaped strings in escaping test --- internal/graph/dotgraph_test.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/internal/graph/dotgraph_test.go b/internal/graph/dotgraph_test.go index 547266692d..e8db65b663 100644 --- a/internal/graph/dotgraph_test.go +++ b/internal/graph/dotgraph_test.go @@ -144,17 +144,24 @@ func TestComposeWithNamesThatNeedEscaping(t *testing.T) { g.Nodes[0].Info = NodeInfo{Name: `var"src"`, File: `/src/file"name.go`, Lineno: 10} g.Nodes[1].Info = NodeInfo{Address: 0x10, Objfile: `/lib/obj"file.so`} c.Title = `test"title` + c.Labels = []string{"legend", `label"text`} var buf bytes.Buffer ComposeDot(&buf, g, a, c) got := buf.String() - for _, s := range []string{`var"src"`, `file"name.go`, `obj"file.so`, `test"title`, `label3: "foo"`} { - if strings.Contains(got, s) { - t.Errorf("unescaped %q in dot output:\n%s", s, got) + for _, tc := range []struct{ raw, escaped string }{ + {`var"src"`, `var\"src\"`}, + {`file"name.go`, `file\"name.go`}, + {`obj"file.so`, `obj\"file.so`}, + {`test"title`, `test\"title`}, + {`label"text`, `label\"text`}, + } { + if strings.Contains(got, tc.raw) { + t.Errorf("unescaped %q in dot output:\n%s", tc.raw, got) } - if want := escapeForDot(s); !strings.Contains(got, want) { - t.Errorf("escaped %q not found in dot output:\n%s", want, got) + if !strings.Contains(got, tc.escaped) { + t.Errorf("escaped %q not found in dot output:\n%s", tc.escaped, got) } } }