From 77951dd7102381385093209a1f2597d28e39900a Mon Sep 17 00:00:00 2001 From: Alexey Botchkov Date: Tue, 25 Oct 2022 23:48:54 +0400 Subject: [PATCH] MDEV-26161 crash in Gis_point::calculate_haversine More checks for bad geometry data added. --- mysql-test/main/gis.result | 4 ++++ mysql-test/main/gis.test | 4 ++++ sql/item_geofunc.cc | 4 ++-- sql/spatial.cc | 32 +++++++++++++++++++++----------- sql/spatial.h | 2 ++ 5 files changed, 33 insertions(+), 13 deletions(-) diff --git a/mysql-test/main/gis.result b/mysql-test/main/gis.result index 3d8c64b0ce8..358be520b06 100644 --- a/mysql-test/main/gis.result +++ b/mysql-test/main/gis.result @@ -4981,6 +4981,10 @@ ERROR HY000: Illegal parameter data type geometry for operation 'is_used_lock' # select st_distance_sphere(x'01030000000400000004000000000000', multipoint(point(124,204)), 10); ERROR 22003: Cannot get geometry object from data you send to the GEOMETRY field +select st_distance_sphere(x'010300000004000000040000', multipoint(point(124,204)), 10); +ERROR 22003: Cannot get geometry object from data you send to the GEOMETRY field +select st_distance_sphere(x'010300000001000000040000', multipoint(point(124,204)), 10); +ERROR 22003: Cannot get geometry object from data you send to the GEOMETRY field # # End of 10.3 tests # diff --git a/mysql-test/main/gis.test b/mysql-test/main/gis.test index 1d202e9be08..716fab9bfeb 100644 --- a/mysql-test/main/gis.test +++ b/mysql-test/main/gis.test @@ -3095,6 +3095,10 @@ SELECT IS_USED_LOCK(POINT(1,1)); --echo # --error ER_CANT_CREATE_GEOMETRY_OBJECT select st_distance_sphere(x'01030000000400000004000000000000', multipoint(point(124,204)), 10); +--error ER_CANT_CREATE_GEOMETRY_OBJECT +select st_distance_sphere(x'010300000004000000040000', multipoint(point(124,204)), 10); +--error ER_CANT_CREATE_GEOMETRY_OBJECT +select st_distance_sphere(x'010300000001000000040000', multipoint(point(124,204)), 10); --echo # --echo # End of 10.3 tests diff --git a/sql/item_geofunc.cc b/sql/item_geofunc.cc index 87c736ab94b..6e65366f2e0 100644 --- a/sql/item_geofunc.cc +++ b/sql/item_geofunc.cc @@ -2656,13 +2656,13 @@ double Item_func_sphere_distance::spherical_distance_points(Geometry *g1, break; } - if (err_hv > 0) + if (err_hv == 1) my_error(ER_STD_OUT_OF_RANGE_ERROR, MYF(0), "Longitude should be [-180,180]", "ST_Distance_Sphere"); else if(err_hv < 0) my_error(ER_STD_OUT_OF_RANGE_ERROR, MYF(0), "Latitude should be [-90,90]", "ST_Distance_Sphere"); - else if (err_sph) + else if (err_sph || err_hv == 2) my_error(ER_CANT_CREATE_GEOMETRY_OBJECT, MYF(0)); return res; } diff --git a/sql/spatial.cc b/sql/spatial.cc index 6e044a67161..9a30d346a1c 100644 --- a/sql/spatial.cc +++ b/sql/spatial.cc @@ -1071,10 +1071,9 @@ double Gis_point::calculate_haversine(const Geometry *g, point_temp[point_size-1]= '\0'; Geometry_buffer gbuff; Geometry *gg= Geometry::construct(&gbuff, point_temp, point_size-1); - DBUG_ASSERT(gg); - if (static_cast(gg)->get_xy_radian(&x2r, &y2r)) + if (!gg || static_cast(gg)->get_xy_radian(&x2r, &y2r)) { - DBUG_ASSERT(0); + *error= 2; return -1; } } @@ -1082,15 +1081,16 @@ double Gis_point::calculate_haversine(const Geometry *g, { if (static_cast(g)->get_xy_radian(&x2r, &y2r)) { - DBUG_ASSERT(0); + *error= 2; return -1; } } if (this->get_xy_radian(&x1r, &y1r)) { - DBUG_ASSERT(0); + *error= 2; return -1; } + // // Check boundary conditions: longitude[-180,180] if (!((x2r >= -M_PI && x2r <= M_PI) && (x1r >= -M_PI && x1r <= M_PI))) { @@ -1143,12 +1143,16 @@ int Gis_point::spherical_distance_multipoints(Geometry *g, const double r, { Geometry_buffer buff_temp; Geometry *temp; + const char *pt_ptr= g->get_data_ptr()+ + 4+WKB_HEADER_SIZE*i + POINT_DATA_SIZE*(i-1); // First 4 bytes are handled already, make sure to create a Point memset(s + 4, Geometry::wkb_point, 1); + if (g->no_data(pt_ptr, POINT_DATA_SIZE)) + return 1; + memcpy(s + 5, g->get_data_ptr() + 5, 4); - memcpy(s + 4 + WKB_HEADER_SIZE, g->get_data_ptr() + 4 + WKB_HEADER_SIZE*i +\ - POINT_DATA_SIZE*(i-1), POINT_DATA_SIZE); + memcpy(s + 4 + WKB_HEADER_SIZE, pt_ptr, POINT_DATA_SIZE); s[len-1]= '\0'; temp= Geometry::construct(&buff_temp, s, len); if (!temp) @@ -2329,11 +2333,14 @@ int Gis_multi_point::spherical_distance_multipoints(Geometry *g, const double r, Geometry *temp; double temp_res= 0.0; char s[len]; + const char *pt_ptr= get_data_ptr()+ + 4+WKB_HEADER_SIZE*i + POINT_DATA_SIZE*(i-1); // First 4 bytes are handled already, make sure to create a Point memset(s + 4, Geometry::wkb_point, 1); + if (no_data(pt_ptr, POINT_DATA_SIZE)) + return 1; memcpy(s + 5, this->get_data_ptr() + 5, 4); - memcpy(s + 4 + WKB_HEADER_SIZE, this->get_data_ptr() + 4 + WKB_HEADER_SIZE*i +\ - POINT_DATA_SIZE*(i-1), POINT_DATA_SIZE); + memcpy(s + 4 + WKB_HEADER_SIZE, pt_ptr, POINT_DATA_SIZE); s[len-1]= '\0'; temp= Geometry::construct(&buff_temp, s, len); if (!temp) @@ -2349,11 +2356,14 @@ int Gis_multi_point::spherical_distance_multipoints(Geometry *g, const double r, Geometry_buffer buff_temp2; Geometry *temp2; char s2[len]; + const char *pt_ptr= g->get_data_ptr()+ + 4+WKB_HEADER_SIZE*j + POINT_DATA_SIZE*(j-1); // First 4 bytes are handled already, make sure to create a Point memset(s2 + 4, Geometry::wkb_point, 1); + if (g->no_data(pt_ptr, POINT_DATA_SIZE)) + return 1; memcpy(s2 + 5, g->get_data_ptr() + 5, 4); - memcpy(s2 + 4 + WKB_HEADER_SIZE, g->get_data_ptr() + 4 + WKB_HEADER_SIZE*j +\ - POINT_DATA_SIZE*(j-1), POINT_DATA_SIZE); + memcpy(s2 + 4 + WKB_HEADER_SIZE, pt_ptr, POINT_DATA_SIZE); s2[len-1]= '\0'; temp2= Geometry::construct(&buff_temp2, s2, len); if (!temp2) diff --git a/sql/spatial.h b/sql/spatial.h index 0c00482c09b..1a69b32bb1c 100644 --- a/sql/spatial.h +++ b/sql/spatial.h @@ -354,6 +354,7 @@ protected: const char *get_mbr_for_points(MBR *mbr, const char *data, uint offset) const; +public: /** Check if there're enough data remaining as requested @@ -384,6 +385,7 @@ protected: (expected_points > ((m_data_end - data) / (POINT_DATA_SIZE + extra_point_space)))); } +protected: const char *m_data; const char *m_data_end; };