Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nef3 test failures in openscad with PR #5371 #5514

Open
leo-lb opened this issue Mar 6, 2021 · 7 comments
Open

nef3 test failures in openscad with PR #5371 #5514

leo-lb opened this issue Mar 6, 2021 · 7 comments

Comments

@leo-lb
Copy link

leo-lb commented Mar 6, 2021

Hello!

Patching CVE-2020-28601, CVE-2020-28636, CVE-2020-35628 and CVE-2020-35636 by applying #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: openscad/openscad#3707

Thank you

@sloriot
Copy link
Member

sloriot commented Mar 9, 2021

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

@leo-lb
Copy link
Author

leo-lb commented Mar 10, 2021

@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
Copy link
Member

sloriot commented Mar 10, 2021

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

@maxGimeno
Copy link
Contributor

maxGimeno commented Mar 10, 2021

I tried to load the file in commit 27de834, 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 ?

@leo-lb
Copy link
Author

leo-lb commented Mar 16, 2021

@maxGimeno guess we should wait for the issue at OpenSCAD's openscad/openscad#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
Copy link

t-paul commented Mar 20, 2021

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
Copy link

pfsmorigo commented Feb 8, 2022

Hello, is there any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants