Ultimaker / CuraEngine

Powerful, fast and robust engine for converting 3D models into g-code instructions for 3D printers. It is part of the larger open source project Cura.
https://ultimaker.com/en/products/cura-software
GNU Affero General Public License v3.0
1.68k stars 883 forks source link

Fix integer underflow on 32 bit systems #1860

Closed codand closed 1 year ago

codand commented 1 year ago

Description

Fixes a possible integer underflow which causes an out of bounds array access when size_t and coord_t are different sizes.

Type of change

How Has This Been Tested?

We build CuraEngine to WebAssembly. Repro steps are a bit more involved than I can easily include here, but I am able to consistently crash the program, and have observed it no longer crashes after this fix.

Test Results (wasm) ``` 12:12:22:~/Workspace/CuraEngine/build/Release % ctest Test project /Users/cody/Workspace/CuraEngine/build/Release Start 1: ClipperTest 1/23 Test #1: ClipperTest ...................... Passed 0.06 sec Start 2: ExtruderPlanTest 2/23 Test #2: ExtruderPlanTest ................. Passed 0.05 sec Start 3: GCodeExportTest 3/23 Test #3: GCodeExportTest .................. Passed 0.05 sec Start 4: InfillTest 4/23 Test #4: InfillTest ....................... Passed 0.05 sec Start 5: LayerPlanTest 5/23 Test #5: LayerPlanTest .................... Passed 0.05 sec Start 6: PathOrderOptimizerTest 6/23 Test #6: PathOrderOptimizerTest ........... Passed 0.05 sec Start 7: PathOrderMonotonicTest 7/23 Test #7: PathOrderMonotonicTest ........... Passed 0.05 sec Start 8: TimeEstimateCalculatorTest 8/23 Test #8: TimeEstimateCalculatorTest ....... Passed 0.05 sec Start 9: WallsComputationTest 9/23 Test #9: WallsComputationTest ............. Passed 0.05 sec Start 10: SlicePhaseTest 10/23 Test #10: SlicePhaseTest ................... Passed 0.05 sec Start 11: SettingsTest 11/23 Test #11: SettingsTest ..................... Passed 0.05 sec Start 12: AABBTest 12/23 Test #12: AABBTest ......................... Passed 0.05 sec Start 13: AABB3DTest 13/23 Test #13: AABB3DTest ....................... Passed 0.05 sec Start 14: IntPointTest 14/23 Test #14: IntPointTest ..................... Passed 0.05 sec Start 15: LinearAlg2DTest 15/23 Test #15: LinearAlg2DTest .................. Passed 0.05 sec Start 16: MinimumSpanningTreeTest 16/23 Test #16: MinimumSpanningTreeTest .......... Passed 0.05 sec Start 17: PolygonConnectorTest 17/23 Test #17: PolygonConnectorTest ............. Passed 0.05 sec Start 18: PolygonTest 18/23 Test #18: PolygonTest ...................... Passed 0.05 sec Start 19: PolygonUtilsTest 19/23 Test #19: PolygonUtilsTest ................. Passed 0.05 sec Start 20: SimplifyTest 20/23 Test #20: SimplifyTest ..................... Passed 0.05 sec Start 21: SparseGridTest 21/23 Test #21: SparseGridTest ................... Passed 0.14 sec Start 22: StringTest 22/23 Test #22: StringTest ....................... Passed 0.11 sec Start 23: UnionFindTest 23/23 Test #23: UnionFindTest .................... Passed 0.05 sec 100% tests passed, 0 tests failed out of 23 Total Test time (real) = 1.34 sec ```

Test Configuration:

Checklist:

rburema commented 1 year ago

@jellespijker Since you asked this month ago, this change is so small and non-offensive, I'll just fix it (to the proper cast) myself & merge it.

rburema commented 1 year ago

Cherry-picked the first commit, changed it to a modern style cast, then merged to main. As such, closing this PR as done, since the main change was merged. (See 2d72ea0c311e237165e28dcbcfd9c797f7aae032 and ddcd384b71f400f80d1b8b711ca84631e140fbd1)