From 072b4f7cf3dd6f31ef4b820de8c938e4108a549f Mon Sep 17 00:00:00 2001 From: Nelson Elhage Date: Thu, 3 May 2018 09:25:30 -0700 Subject: [PATCH] agnameof: Use agstrdup Callers expect the returned string to be safe to pass to other `refstr` functions; Make sure we return a refstr, and prevent an out-of-bounds read. Found via ASAN+libfuzzer, reproducible as: ``` $ work/bin/dot -Tdot <(echo 'graph{"%a"}') graph { graph [bb="0,0,54,36"]; node [label="\N"]; ================================================================= ==5889==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7fa16b46b4f0 at pc 0x7fa16b24cd8f bp 0x7ffcded43f70 sp 0x7ffcded43f68 READ of size 8 at 0x7fa16b46b4f0 thread T0 #0 0x7fa16b24cd8e in aghtmlstr /home/nelhage/code/graphviz/lib/cgraph/refstr.c:185:18 #1 0x7fa16b25a008 in agstrcanon /home/nelhage/code/graphviz/lib/cgraph/write.c:179:9 #2 0x7fa16b25b3eb in agcanonStr /home/nelhage/code/graphviz/lib/cgraph/write.c:209:12 #3 0x7fa16b25cdb9 in _write_canonstr /home/nelhage/code/graphviz/lib/cgraph/write.c:230:8 #4 0x7fa16b25cbc9 in write_canonstr /home/nelhage/code/graphviz/lib/cgraph/write.c:238:12 #5 0x7fa16b25e17b in write_nodename /home/nelhage/code/graphviz/lib/cgraph/write.c:522:2 #6 0x7fa16b25d6d9 in write_node /home/nelhage/code/graphviz/lib/cgraph/write.c:541:5 #7 0x7fa16b25c418 in write_body /home/nelhage/code/graphviz/lib/cgraph/write.c:640:6 #8 0x7fa16b25bb91 in agwrite /home/nelhage/code/graphviz/lib/cgraph/write.c:690:5 #9 0x7fa166c8c216 in dot_end_graph /home/nelhage/code/graphviz/plugin/core/gvrender_core_dot.c:522:3 #10 0x7fa16b517057 in gvrender_end_graph /home/nelhage/code/graphviz/lib/gvc/gvrender.c:258:6 #11 0x7fa16b6a9448 in emit_end_graph /home/nelhage/code/graphviz/lib/common/emit.c:3464:5 #12 0x7fa16b6a6540 in emit_graph /home/nelhage/code/graphviz/lib/common/emit.c:3599:5 #13 0x7fa16b6afee7 in gvRenderJobs /home/nelhage/code/graphviz/lib/common/emit.c:4198:6 #14 0x512b37 in main /home/nelhage/code/graphviz/cmd/dot/dot.c:133:6 #15 0x7fa16a274b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310 #16 0x41a499 in _start (/home/nelhage/code/graphviz/work/bin/dot+0x41a499) 0x7fa16b46b4f0 is located 16 bytes to the left of global variable 'buf' defined in 'id.c:147:17' (0x7fa16b46b500) of size 32 0x7fa16b46b4f0 is located 44 bytes to the right of global variable 'req' defined in '../../lib/cgraph/grammar.y:542:18' (0x7fa16b46b4c0) of size 4 SUMMARY: AddressSanitizer: global-buffer-overflow /home/nelhage/code/graphviz/lib/cgraph/refstr.c:185:18 in aghtmlstr ``` --- lib/cgraph/id.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/cgraph/id.c b/lib/cgraph/id.c index 694220ef39..c37c7e590e 100644 --- a/lib/cgraph/id.c +++ b/lib/cgraph/id.c @@ -138,7 +138,6 @@ void agfreeid(Agraph_t * g, int objtype, IDTYPE id) * Return string representation of object. * In general, returns the name of node or graph, * and the key of an edge. If edge is anonymous, returns NULL. - * Uses static buffer for anonymous graphs. */ char *agnameof(void *obj) { @@ -158,7 +157,7 @@ char *agnameof(void *obj) } if (AGTYPE(obj) != AGEDGE) { sprintf(buf, "%c%ld", LOCALNAMEPREFIX, AGID(obj)); - rv = buf; + rv = agstrdup(g, buf); } else rv = 0; -- GitLab