search

CGAL / cgal

The public CGAL repository, see the README below
https://github.com/CGAL/cgal#readme
Other
4.86k stars 1.38k forks source link

nef3 test failures in openscad with PR #5371 #5514

Open llebout opened 3 years ago

llebout commented 3 years ago

Hello!

Patching CVE-2020-28601, CVE-2020-28636, CVE-2020-35628 and CVE-2020-35636 by applying https://github.com/CGAL/cgal/pull/5371 on CGAL 5.2 in GNU Guix started causing nef3 related test failures on openscad. It is not certain what is the cause of these failures, if the PR to fix the security issue is wrong, or the openscad tests.

Please see the openscad issue as well: https://github.com/openscad/openscad/issues/3707

Thank you

sloriot commented 3 years ago

@leo-lb any chance you can provide at least one .nef file or is it buried in the openscad pipeline?

llebout commented 3 years ago

@sloriot That's where they are, apparently, wasnt sure if that was openscad-specific or not: https://raw.githubusercontent.com/openscad/openscad/openscad-2021.01/testdata/nef3/broken.nef3

sloriot commented 3 years ago

@maxGimeno could you please try to load this file before and after your patch?

maxGimeno commented 3 years ago

I tried to load the file in commit 27de834e3004b2eab757dc940f108d7f3021bad9, which is just before the first commit of my Fix, and there is an assertion. But it seems normal, as the file is wrong, there is a letter instead of a number in the failing edge. What would be the expected behavior, if not an assertion ?

llebout commented 3 years ago

@maxGimeno guess we should wait for the issue at OpenSCAD's https://github.com/openscad/openscad/issues/3707 to get an answer so they can fix their presumably broken test

I don't remember exactly but I think I tried CGAL 5.2 with your PR fix and it caused test failures while without it it passed, so you say even without your fixes it should still fail? Will have to re-test since I kept no log of this.

t-paul commented 3 years ago

I don't see any way to fix the issue on OpenSCAD side. This is really just calling the << operator on Nef_polyhedron_3 and then crashing while reading the file. The whole point of the test case is that reading a broken file is safe, so the nef3 file is broken, the test case is not.

https://github.com/openscad/openscad/blob/4ebebdaf0dec2f7aaebb6a1c1f6163f503caaa63/src/import_nef.cc#L29-L31

pfsmorigo commented 2 years ago

Hello, is there any update on this?