FreeCADView Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003070 | File formats | General | public | 2017-06-05 01:02 | 2021-02-06 06:31 |
| Reporter | Krishty | Assigned To | |||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | confirmed | Resolution | open | ||
| Target Version | 0.20 | ||||
| Summary | 0003070: VRML 2 crashes with corner cases | ||||
| Description | 1. FreeCAD crashes whenever VRML files contain recursive structures. 2. It crashes after some time if files contain very long names. Recursive structures are not allowed in VRML, and they should not appear in the wild. However, FreeCAD shouldn't just crash either. Same for long names. | ||||
| Steps To Reproduce | 1. load one of the attached files with "crash" prefix 2. crash immediately OR 1. click Open File 2. select the file with "corrupts memory" prefix 3. Open 4. file seems to load file 5. click Open File again 6. FreeCAD crashes a fraction of a second after that (which indicates a memory corruption) | ||||
| Additional Information | running 0.16 on Windows 7 | ||||
| Tags | VRML | ||||
| FreeCAD Information | |||||
|
|
|
|
|
same crashes with VRML 1 recursive structures & VRML 1 long name |
|
|
Discussion thread: https://forum.freecadweb.org/viewtopic.php?f=8&t=22875 |
|
|
<quote> 2. It crashes after some time if files contain very long names. </quote> That's a bug in the Coin library. Inside the class SbName the function find_string_address is called (over several steps) and this raises an assert if the string is longer than 65004 characters. Many systems already crash there (when Coin was built with the debug option). On Windows in release mode it crashes at a later point because the internal string buffer seems to be corrupted. Does the VRML standard say anything about the maximum length of a name? |
|
|
The VRML 2 standard defines a limit of 50 Bytes per name: http://tecfa.unige.ch/guides/vrml/vrml97/spec/part1/conformance.html#7.3.3 VRML 1 standard does not mention anything. I recommend aborting after 50 B if you’re strict or after 127 B if you want to avoid problems with 3rd party libraries. |
|
|
Thanks for the link. > I recommend aborting after 50 B if you’re strict or after 127 B if you want to avoid problems with 3rd party libraries. Currently we rely on Coin's VRML importer here. So as a workaround we must implement our own parser that verifies the DEF's of a VRML file. |
|
|
Yes, a separate verification pass would also solve the recursive DEF/USE crash. However, given that VRML 1 & 2 are vastly different in grammar and the bug exists in both versions, that’s probably TWO parsers to be written. It should definitely be reported to the COIN maintainers and they need to fix that. |
|
|
I pushed a fix https://github.com/FreeCAD/FreeCAD/commit/a405b4dae05fc55d784cc485723f278b83ea64ee the solves the crash when loading the file crash - recursive USE.wrl Having this function inside FreeCAD makes sense because cyclic graphs can also happen outside the scope of reading in a file. The fix doesn't prevent FreeCAD from crashing when reading in crash - recursive PROTO.wrl because there is already a stack overflow during the read process. Btw, MeshLab handles the cases with recursive PROTO and too long names but crashes with recursive USE. Blender handles all three cases. |
|
|
> Btw, MeshLab handles the cases with recursive PROTO and too long names but crashes with recursive USE. Blender handles all three cases. Good thing to know! I don’t actually use FreeCAD, I was just fuzz-testing my own VRML implementation and wondered how other software suites do (considering we live in the age of exploits). It’s just fair to give other developers every chance to improve their software, so I reported the crashes outright. You should report them to MeshLab as well; use my test files at will. |
|
|
This ticket has been migrated to GitHub as issue 5707. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2017-06-05 01:02 | Krishty | New Issue | |
| 2017-06-05 01:02 | Krishty | File Added: corrupts memory - name too long.wrl | |
| 2017-06-05 01:02 | Krishty | File Added: crash - recursive PROTO.wrl | |
| 2017-06-05 01:02 | Krishty | File Added: crash - recursive USE.wrl | |
| 2017-06-05 01:10 | Krishty | File Added: corrupts memory - name too long (VRML 1).wrl | |
| 2017-06-05 01:10 | Krishty | File Added: crash - recursive USE (VRML 1).wrl | |
| 2017-06-05 01:10 | Krishty | Note Added: 0009285 | |
| 2017-06-10 05:13 | Kunda1 | Tag Attached: VRML | |
| 2017-06-10 11:35 | Kunda1 | Note Added: 0009331 | |
| 2017-06-11 12:16 | wmayer | Note Added: 0009343 | |
| 2017-06-11 12:33 | Krishty | Note Added: 0009344 | |
| 2017-06-11 12:46 | wmayer | Note Added: 0009345 | |
| 2017-06-11 13:01 | Krishty | Note Added: 0009346 | |
| 2017-06-11 14:49 | wmayer | Note Added: 0009347 | |
| 2017-06-11 14:51 | wmayer | Status | new => confirmed |
| 2017-06-11 15:36 | wmayer | Note Edited: 0009347 | |
| 2017-06-11 18:09 | Krishty | Note Added: 0009348 | |
| 2021-02-06 06:31 | abdullah | Target Version | => 0.20 |
