Skip to content

Commit 788c53e

Browse files
authored
Avoid accessing vectors with zero length, by checking data sizes. (#5708)
* Avoid accessing vectors with zero length, by checking data sizes. * Use empty rather than size() > 0. * Update per review suggestions. Remove setting info fields in pcd_io.h but instead check for msg.data.size in fromPCLPointCloud2, where fields are set. Remove unnecessary check in readHeader. * Use msg width and height to determine if there are points. �Readd warning in PCDReader::readHeader. * Return after resizing output cloud. Use .data instead of acquiring address of index 0.
1 parent cac9db8 commit 788c53e

File tree

3 files changed

+54
-38
lines changed

3 files changed

+54
-38
lines changed

common/include/pcl/conversions.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,18 @@ namespace pcl
169169
cloud.height = msg.height;
170170
cloud.is_dense = msg.is_dense == 1;
171171

172-
// Copy point data
172+
// Resize cloud
173173
cloud.resize (msg.width * msg.height);
174-
std::uint8_t* cloud_data = reinterpret_cast<std::uint8_t*>(&cloud[0]);
174+
175+
// check if there is data to copy
176+
if (msg.width * msg.height == 0)
177+
{
178+
PCL_WARN("[pcl::fromPCLPointCloud2] No data to copy.\n");
179+
return;
180+
}
181+
182+
// Copy point data
183+
std::uint8_t* cloud_data = reinterpret_cast<std::uint8_t*>(cloud.data());
175184

176185
// Check if we can copy adjacent points in a single memcpy. We can do so if there
177186
// is exactly one field to copy and it is the same size as the source and destination
@@ -272,7 +281,7 @@ namespace pcl
272281
msg.data.resize (data_size);
273282
if (data_size)
274283
{
275-
memcpy(&msg.data[0], &cloud[0], data_size);
284+
memcpy(msg.data.data(), cloud.data(), data_size);
276285
}
277286

278287
// Fill fields metadata

io/include/pcl/io/pcd_io.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ namespace pcl
284284
// If no error, convert the data
285285
if (res == 0)
286286
pcl::fromPCLPointCloud2 (blob, cloud);
287+
287288
return (res);
288289
}
289290

io/src/pcd_io.cpp

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -341,9 +341,9 @@ pcl::PCDReader::readHeader (std::istream &fs, pcl::PCLPointCloud2 &cloud,
341341

342342
if (nr_points == 0)
343343
{
344-
PCL_WARN ("[pcl::PCDReader::readHeader] No points to read\n");
344+
PCL_WARN("[pcl::PCDReader::readHeader] number of points is zero.\n");
345345
}
346-
346+
347347
// Compatibility with older PCD file versions
348348
if (!width_read && !height_read)
349349
{
@@ -557,6 +557,11 @@ pcl::PCDReader::readBodyBinary (const unsigned char *map, pcl::PCLPointCloud2 &c
557557
}
558558

559559
auto data_size = static_cast<unsigned int> (cloud.data.size ());
560+
if (data_size == 0)
561+
{
562+
PCL_WARN("[pcl::PCDReader::read] Binary compressed file has data size of zero.\n");
563+
return 0;
564+
}
560565
std::vector<char> buf (data_size);
561566
// The size of the uncompressed data better be the same as what we stored in the header
562567
unsigned int tmp_size = pcl::lzfDecompress (&map[data_idx + 8], compressed_size, buf.data(), data_size);
@@ -1369,42 +1374,43 @@ pcl::PCDWriter::writeBinaryCompressed (std::ostream &os, const pcl::PCLPointClou
13691374
return (-2);
13701375
}
13711376

1372-
//////////////////////////////////////////////////////////////////////
1373-
// Empty array holding only the valid data
1374-
// data_size = nr_points * point_size
1375-
// = nr_points * (sizeof_field_1 + sizeof_field_2 + ... sizeof_field_n)
1376-
// = sizeof_field_1 * nr_points + sizeof_field_2 * nr_points + ... sizeof_field_n * nr_points
1377-
std::vector<char> only_valid_data (data_size);
1378-
1379-
// Convert the XYZRGBXYZRGB structure to XXYYZZRGBRGB to aid compression. For
1380-
// this, we need a vector of fields.size () (4 in this case), which points to
1381-
// each individual plane:
1382-
// pters[0] = &only_valid_data[offset_of_plane_x];
1383-
// pters[1] = &only_valid_data[offset_of_plane_y];
1384-
// pters[2] = &only_valid_data[offset_of_plane_z];
1385-
// pters[3] = &only_valid_data[offset_of_plane_RGB];
1386-
//
1387-
std::vector<char*> pters (fields.size ());
1388-
std::size_t toff = 0;
1389-
for (std::size_t i = 0; i < pters.size (); ++i)
1390-
{
1391-
pters[i] = &only_valid_data[toff];
1392-
toff += fields_sizes[i] * cloud.width * cloud.height;
1393-
}
1377+
std::vector<char> temp_buf (data_size * 3 / 2 + 8);
1378+
if (data_size != 0) {
13941379

1395-
// Go over all the points, and copy the data in the appropriate places
1396-
for (uindex_t i = 0; i < cloud.width * cloud.height; ++i)
1397-
{
1398-
for (std::size_t j = 0; j < pters.size (); ++j)
1399-
{
1400-
memcpy (pters[j], &cloud.data[i * cloud.point_step + fields[j].offset], fields_sizes[j]);
1401-
// Increment the pointer
1402-
pters[j] += fields_sizes[j];
1380+
//////////////////////////////////////////////////////////////////////
1381+
// Empty array holding only the valid data
1382+
// data_size = nr_points * point_size
1383+
// = nr_points * (sizeof_field_1 + sizeof_field_2 + ... sizeof_field_n)
1384+
// = sizeof_field_1 * nr_points + sizeof_field_2 * nr_points + ...
1385+
// sizeof_field_n * nr_points
1386+
std::vector<char> only_valid_data(data_size);
1387+
1388+
// Convert the XYZRGBXYZRGB structure to XXYYZZRGBRGB to aid compression. For
1389+
// this, we need a vector of fields.size () (4 in this case), which points to
1390+
// each individual plane:
1391+
// pters[0] = &only_valid_data[offset_of_plane_x];
1392+
// pters[1] = &only_valid_data[offset_of_plane_y];
1393+
// pters[2] = &only_valid_data[offset_of_plane_z];
1394+
// pters[3] = &only_valid_data[offset_of_plane_RGB];
1395+
//
1396+
std::vector<char*> pters(fields.size());
1397+
std::size_t toff = 0;
1398+
for (std::size_t i = 0; i < pters.size(); ++i) {
1399+
pters[i] = &only_valid_data[toff];
1400+
toff += fields_sizes[i] * cloud.width * cloud.height;
1401+
}
1402+
1403+
// Go over all the points, and copy the data in the appropriate places
1404+
for (uindex_t i = 0; i < cloud.width * cloud.height; ++i) {
1405+
for (std::size_t j = 0; j < pters.size(); ++j) {
1406+
memcpy(pters[j],
1407+
&cloud.data[i * cloud.point_step + fields[j].offset],
1408+
fields_sizes[j]);
1409+
// Increment the pointer
1410+
pters[j] += fields_sizes[j];
1411+
}
14031412
}
1404-
}
14051413

1406-
std::vector<char> temp_buf (data_size * 3 / 2 + 8);
1407-
if (data_size != 0) {
14081414
// Compress the valid data
14091415
unsigned int compressed_size = pcl::lzfCompress (&only_valid_data.front (),
14101416
static_cast<unsigned int> (data_size),

0 commit comments

Comments
 (0)