[text mode]ParseShape(SVGNative::XMLNode*): Assertion `rx == ry' failed.

[hardik05]

found one issue where program terminates because of assert failure, following is the log:
pwndbg> r /home/test/svg-native-viewer/svgnative/Build/linux/example/testText/output/Slave/crashes/poc.svg 1
Starting program: /home/test/svg-native-viewer/svgnative/Build/linux/example/testText/testSVGNative /home/test/svg-native-viewer/svgnative/Build/linux/example/testText/output/Slave/crashes/poc.svg 1
testSVGNative: /home/test/svg-native-viewer/svgnative/src/SVGDocumentImpl.cpp:511: std::unique_ptrSVGNative::Path SVGNative::SVGDocumentImpl::ParseShape(SVGNative::XMLNode*): Assertion `rx == ry' failed.

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
LEGEND: STACK | HEAP | CODE | DATA | RWX | RODATA
───────────────────────────────────────────────────────────[ REGISTERS ]───────────────────────────────────────────────────────────
RAX 0x0
RBX 0x0
RCX 0x7ffff70e3e97 (raise+199) ◂— mov rcx, qword ptr [rsp + 0x108]
RDX 0x0
RDI 0x2
RSI 0x7fffffffd460 ◂— 0x0
R8 0x0
R9 0x7fffffffd460 ◂— 0x0
R10 0x8
R11 0x246
R12 0x555555619310 ◂— '/home/test/svg-native-viewer/svgnative/src/SVGDocumentImpl.cpp'
R13 0x5555556193b3 ◂— jb 0x55555561942d /* 'rx == ry' /
R14 0x1ff
R15 0x0
RBP 0x7ffff725c7d8 ◂— and eax, 0x25732573 / "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n" */
RSP 0x7fffffffd460 ◂— 0x0
RIP 0x7ffff70e3e97 (raise+199) ◂— mov rcx, qword ptr [rsp + 0x108]
────────────────────────────────────────────────────────────[ DISASM ]─────────────────────────────────────────────────────────────
► 0x7ffff70e3e97 <raise+199> mov rcx, qword ptr [rsp + 0x108] <0x7ffff70e3e97>
0x7ffff70e3e9f <raise+207> xor rcx, qword ptr fs:[0x28]
0x7ffff70e3ea8 <raise+216> mov eax, r8d
0x7ffff70e3eab <raise+219> jne raise+252 <0x7ffff70e3ecc>
↓
0x7ffff70e3ecc <raise+252> call __stack_chk_fail <0x7ffff71d9c80>

0x7ffff70e3ed1 nop word ptr cs:[rax + rax]
0x7ffff70e3edb nop dword ptr [rax + rax]
0x7ffff70e3ee0 test edi, edi
0x7ffff70e3ee2 <killpg+2> js killpg+16 <0x7ffff70e3ef0>

0x7ffff70e3ee4 <killpg+4> neg edi
0x7ffff70e3ee6 <killpg+6> jmp kill <0x7ffff70e4180>
─────────────────────────────────────────────────────────────[ STACK ]─────────────────────────────────────────────────────────────
00:0000│ rsi r9 rsp 0x7fffffffd460 ◂— 0x0
01:0008│ 0x7fffffffd468 —▸ 0x555555836da0 ◂— 0x0
02:0010│ 0x7fffffffd470 ◂— 0x0
03:0018│ 0x7fffffffd478 —▸ 0x7ffff712d5e0 (vasprintf+240) ◂— test rax, rax
04:0020│ 0x7fffffffd480 ◂— 0xfbad8000
05:0028│ 0x7fffffffd488 —▸ 0x555555836da0 ◂— 0x0
06:0030│ 0x7fffffffd490 —▸ 0x555555836e05 ◂— "rSVGNative::Path SVGNative::SVGDocumentImpl::ParseShape(SVGNative::XMLNode*): Assertion rx == ry' failed.\n" 07:0038│ 0x7fffffffd498 —▸ 0x555555836da0 ◂— 0x0 ───────────────────────────────────────────────────────────[ BACKTRACE ]─────────────────────────────────────────────────────────── ► f 0 7ffff70e3e97 raise+199 f 1 7ffff70e5801 abort+321 f 2 7ffff70d539a __assert_fail_base+330 f 3 7ffff70d5412 f 4 5555555a40fb SVGNative::SVGDocumentImpl::ParseShape(boost::property_tree::detail::rapidxml::xml_node<char>*)+5051 f 5 5555555b5a2d SVGNative::SVGDocumentImpl::ParseChild(boost::property_tree::detail::rapidxml::xml_node<char>*)+749 f 6 5555555bb96b SVGNative::SVGDocumentImpl::ParseChildren(boost::property_tree::detail::rapidxml::xml_node<char>*)+155 f 7 5555555bbeb2 SVGNative::SVGDocumentImpl::TraverseSVGTree()+1058 f 8 555555578858 f 9 555555562b54 main+2068 f 10 7ffff70c6b97 __libc_start_main+231 ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Program received signal SIGABRT pwndbg> bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x00007ffff70e5801 in __GI_abort () at abort.c:79 #2 0x00007ffff70d539a in __assert_fail_base (fmt=0x7ffff725c7d8 "%s%s%s:%u: %s%sAssertion %s' failed.\n%n", assertion=assertion@entry=0x5555556193b3 "rx == ry", file=file@entry=0x555555619310 "/home/test/svg-native-viewer/svgnative/src/SVGDocumentImpl.cpp", line=line@entry=511, function=function@entry=0x555555619ae0 <SVGNative::SVGDocumentImpl::ParseShape(boost::property_tree::detail::rapidxml::xml_node)::PRETTY_FUNCTION> "std::unique_ptrSVGNative::Path SVGNative::SVGDocumentImpl::ParseShape(SVGNative::XMLNode)") at assert.c:92
#3 0x00007ffff70d5412 in __GI___assert_fail (assertion=assertion@entry=0x5555556193b3 "rx == ry", file=file@entry=0x555555619310 "/home/test/svg-native-viewer/svgnative/src/SVGDocumentImpl.cpp", line=line@entry=511, function=function@entry=0x555555619ae0 <SVGNative::SVGDocumentImpl::ParseShape(boost::property_tree::detail::rapidxml::xml_node)::PRETTY_FUNCTION> "std::unique_ptrSVGNative::Path SVGNative::SVGDocumentImpl::ParseShape(SVGNative::XMLNode)") at assert.c:101
#4 0x00005555555a40fb in SVGNative::SVGDocumentImpl::ParseShape (this=this@entry=0x555555841240, child=child@entry=0x555555841658) at /home/test/svg-native-viewer/svgnative/src/SVGDocumentImpl.cpp:511
#5 0x00005555555b5a2d in SVGNative::SVGDocumentImpl::ParseChild (this=this@entry=0x555555841240, child=child@entry=0x555555841658) at /home/test/svg-native-viewer/svgnative/src/SVGDocumentImpl.cpp:163
#6 0x00005555555bb96b in SVGNative::SVGDocumentImpl::ParseChildren (this=0x555555841240, node=) at /home/test/svg-native-viewer/svgnative/src/SVGDocumentImpl.cpp:146
#7 0x00005555555bbeb2 in SVGNative::SVGDocumentImpl::TraverseSVGTree (this=this@entry=0x555555841240) at /home/test/svg-native-viewer/svgnative/src/SVGDocumentImpl.cpp:94
#8 0x0000555555578858 in SVGNative::SVGDocument::CreateSVGDocument (s=, renderer=...) at /home/test/svg-native-viewer/svgnative/src/SVGDocument.cpp:37
#9 0x0000555555562b54 in main (argc=argc@entry=3, argv=argv@entry=0x7fffffffe3c8) at /home/test/svg-native-viewer/svgnative/example/testText/TestMain.cpp:52
#10 0x00007ffff70c6b97 in __libc_start_main (main=0x555555562340 <main(int, char* const*)>, argc=3, argv=0x7fffffffe3c8, init=, fini=, rtld_fini=, stack_end=0x7fffffffe3b8) at ../csu/libc-start.c:310
#11 0x0000555555563a0a in _start ()

i can send you the poc via email if needed. Please let me know.

[mpsuzuki]

@hardik05 Many many thank you for finding this unexpected abort by the assertion.

I think, the problem comes from a mismatch of the assumed cases.
In the code for "rect" object in std::unique_ptr SVGDocumentImpl::ParseShape(), rx and ry are calculated like:

if (hasRx && hasRy) {
    rx = ...
    ry = ...
} else if (hasRx) {
    rx = ...;
    ry = rx;
} else if (hasRy) {
    ry = ...;
    rx = ry;
} else {
    rx = 0;
    ry = 0;
}

Thus, there must be a case that rx and ry are different (by explicit declaration)
But, when the rx and ry are checked, this case is overlooked, like,

if (rx == 0 && ry == 0)
    ...
else
    ASSERT(rx == ry)

I would try to tweak the case for assertion. Or, after the assertion, there is "std::max(rx, ry)", so another option is removal of the assertion itself.

If possible, please send me POC file, to mpsuzuki@hiroshima-u.ac.jp .

[mpsuzuki]

@hardik05 Oops, comparing the spec and implementation, I understand you raised more important issue. SVG spec have 2 individual parameters for rounded-rect, rx and ry. But, in the internal API (between the parser and graphic backends) of SVG Native Viewer, there is only "r" for rounded-rect. I should consider whether we should extend, or SVG Native spec restrict as they should be same.

[mpsuzuki]

@hardik05 Sorry for lated response. I drafted my patch (a better support of rounded-rectangle, with different rx and ry) at abfb9aa. Not only string port but also CoreGraphics, Skia, GDI+ and Cairo ports are extended.

[mpsuzuki]

Also I found another issue related this. The "rect" element with rx==0 && ry > 0 (I mean, the case both are explicitly specified, like <rect ... rx="0" ry="10"/>) must be drawn as non-rounded rectangle (it is reasonable). Thus, if (rx == 0 && ry == 0) in SVGDocumentImpl.cpp has a small room of the improvement. if (rx == 0 || ry == 0) would be sufficient, as far as we can distinguish such case from rx-only or ry-only cases.

[dirkschulze]

@mpsuzuki The if condition is correct. Only use simplified rect if both are 0, rx and ry. The assertion is incorrect though.