CVE-2022-0496: Out-of-bounds Read (CWE-125)
value: A vulnerbiility was found in Openscad, where a DXF-format drawing with particular (not necessarily malformed!) properties may cause an out-of-bounds memory access when imported using import().
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2022-08-29T15:15Z
Repository: https://github.com/openscad/openscad
Language: C++
Files changed: src/dxfdata.cc
Commit date: 2022-02-05 00:38:03 +0100
Commit message: add safety to line lookups in DXF import, fixes #4037

Add safety (test for, and continue past, bad indices).
Report warnings about bad indices
Add variables just to make the array indices easier to read and debug.

Diff: diff --git a/src/dxfdata.cc b/src/dxfdata.cc
index 2bb72367..aa6b6f39 100644
--- a/src/dxfdata.cc
+++ b/src/dxfdata.cc
@@ -439,10 +439,15 @@ DxfData::DxfData(double fn, double fs, double fa,
 			int idx = l.second;
 			for (int j = 0; j < 2; ++j) {
 				auto lv = grid.data(this->points[lines[idx].idx[j]][0], this->points[lines[idx].idx[j]][1]);
 				for (size_t ki = 0; ki < lv.size(); ++ki) {
 					int k = lv.at(ki);
+                    if (k < 0 || k >= lines.size()) {
+                        LOG(message_group::Warning,Location::NONE,"",
+                            "Bad DXF line index in %1$s.",QuotedString(boostfs_uncomplete(filename, fs::current_path()).generic_string()));
+                        continue;
+                    }
 					if (k == idx || lines[k].disabled) continue;
 					goto next_open_path_j;
 				}
 				current_line = idx;
 				current_point = j;
@@ -464,17 +469,24 @@ DxfData::DxfData(double fn, double fs, double fa,
 			lines[current_line].disabled = true;
 			enabled_lines.erase(current_line);
 			auto lv = grid.data(ref_point[0], ref_point[1]);
 			for (size_t ki = 0; ki < lv.size(); ++ki) {
 				int k = lv.at(ki);
+                if (k < 0 || k >= lines.size()) {
+                    LOG(message_group::Warning,Location::NONE,"",
+                        "Bad DXF line index in %1$s.",QuotedString(boostfs_uncomplete(filename, fs::current_path()).generic_string()));
+                    continue;
+                }
 				if (lines[k].disabled) continue;
-				if (grid.eq(ref_point[0], ref_point[1], this->points[lines[k].idx[0]][0], this->points[lines[k].idx[0]][1])) {
+                auto idk0 = lines[k].idx[0];    // make it easier to read and debug
+                auto idk1 = lines[k].idx[1];
+				if (grid.eq(ref_point[0], ref_point[1], this->points[idk0][0], this->points[idk0][1])) {
 					current_line = k;
 					current_point = 0;
 					goto found_next_line_in_open_path;
 				}
-				if (grid.eq(ref_point[0], ref_point[1], this->points[lines[k].idx[1]][0], this->points[lines[k].idx[1]][1])) {
+				if (grid.eq(ref_point[0], ref_point[1], this->points[idk1][0], this->points[idk1][1])) {
 					current_line = k;
 					current_point = 1;
 					goto found_next_line_in_open_path;
 				}
 			}
@@ -499,17 +511,24 @@ DxfData::DxfData(double fn, double fs, double fa,
 			lines[current_line].disabled = true;
 			enabled_lines.erase(current_line);
 			auto lv = grid.data(ref_point[0], ref_point[1]);
 			for (size_t ki = 0; ki < lv.size(); ++ki) {
 				int k = lv.at(ki);
+                if (k < 0 || k >= lines.size()) {
+                    LOG(message_group::Warning,Location::NONE,"",
+                        "Bad DXF line index in %1$s.",QuotedString(boostfs_uncomplete(filename, fs::current_path()).generic_string()));
+                    continue;
+                }
 				if (lines[k].disabled) continue;
-				if (grid.eq(ref_point[0], ref_point[1], this->points[lines[k].idx[0]][0], this->points[lines[k].idx[0]][1])) {
+                auto idk0 = lines[k].idx[0];    // make it easier to read and debug
+                auto idk1 = lines[k].idx[1];
+				if (grid.eq(ref_point[0], ref_point[1], this->points[idk0][0], this->points[idk0][1])) {
 					current_line = k;
 					current_point = 0;
 					goto found_next_line_in_closed_path;
 				}
-					if (grid.eq(ref_point[0], ref_point[1], this->points[lines[k].idx[1]][0], this->points[lines[k].idx[1]][1])) {
+                if (grid.eq(ref_point[0], ref_point[1], this->points[idk1][0], this->points[idk1][1])) {
 					current_line = k;
 					current_point = 1;
 					goto found_next_line_in_closed_path;
 				}
 			}

Vulnerable code: 				if (grid.eq(ref_point[0], ref_point[1], this->points[lines[k].idx[0]][0], this->points[lines[k].idx[0]][1])) {
				if (grid.eq(ref_point[0], ref_point[1], this->points[lines[k].idx[1]][0], this->points[lines[k].idx[1]][1])) {
				if (grid.eq(ref_point[0], ref_point[1], this->points[lines[k].idx[0]][0], this->points[lines[k].idx[0]][1])) {
					if (grid.eq(ref_point[0], ref_point[1], this->points[lines[k].idx[1]][0], this->points[lines[k].idx[1]][1])) {
Fixed code:                     if (k < 0 || k >= lines.size()) {
                        LOG(message_group::Warning,Location::NONE,"",
                            "Bad DXF line index in %1$s.",QuotedString(boostfs_uncomplete(filename, fs::current_path()).generic_string()));
                        continue;
                    }
                if (k < 0 || k >= lines.size()) {
                    LOG(message_group::Warning,Location::NONE,"",
                        "Bad DXF line index in %1$s.",QuotedString(boostfs_uncomplete(filename, fs::current_path()).generic_string()));
                    continue;
                }
                auto idk0 = lines[k].idx[0];    // make it easier to read and debug
                auto idk1 = lines[k].idx[1];
				if (grid.eq(ref_point[0], ref_point[1], this->points[idk0][0], this->points[idk0][1])) {
				if (grid.eq(ref_point[0], ref_point[1], this->points[idk1][0], this->points[idk1][1])) {
                if (k < 0 || k >= lines.size()) {
                    LOG(message_group::Warning,Location::NONE,"",
                        "Bad DXF line index in %1$s.",QuotedString(boostfs_uncomplete(filename, fs::current_path()).generic_string()));
                    continue;
                }
                auto idk0 = lines[k].idx[0];    // make it easier to read and debug
                auto idk1 = lines[k].idx[1];
				if (grid.eq(ref_point[0], ref_point[1], this->points[idk0][0], this->points[idk0][1])) {
                if (grid.eq(ref_point[0], ref_point[1], this->points[idk1][0], this->points[idk1][1])) {