Skip to content

Commit 70fa51a

Browse files
IlanTruanovskypcolberg
authored andcommitted
Fix OVERRUN Coverity issues
lib/pkg_editor/src/pkg_editor.c:1641:5: Type: Out-of-bounds read (OVERRUN) lib/pkg_editor/src/pkg_editor.c:1584:3: 1. path: Condition "buffer != NULL", taking true branch. lib/pkg_editor/src/pkg_editor.c:1585:5: 2. path: Condition "input == NULL", taking true branch. lib/pkg_editor/src/pkg_editor.c:1585:5: 3. path: Falling through to end of if statement. lib/pkg_editor/src/pkg_editor.c:1588:3: 4. path: Falling through to end of if statement. lib/pkg_editor/src/pkg_editor.c:1594:3: 5. path: Condition "ret != 0", taking false branch. lib/pkg_editor/src/pkg_editor.c:1610:3: 6. path: Condition "z_info.strm.avail_in > 0", taking true branch. lib/pkg_editor/src/pkg_editor.c:1612:5: 7. path: Condition "!read_data(&info, 20UL /* sizeof (info) */, &z_info, input)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1617:5: 8. path: Condition "info.magic != 3203399403U", taking false branch. lib/pkg_editor/src/pkg_editor.c:1625:5: 9. path: Condition "info.kind == PACK_END", taking false branch. lib/pkg_editor/src/pkg_editor.c:1630:5: 10. path: Condition "!read_data(name, info.name_length, &z_info, input)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1638:5: 11. path: Condition "12288UL /* 3 * 4096 */ < out_dir_length", taking true branch. lib/pkg_editor/src/pkg_editor.c:1638:5: 12. cond_at_least: Checking "12288UL < out_dir_length" implies that "out_dir_length" is at least 12289 on the true branch. lib/pkg_editor/src/pkg_editor.c:1641:5: 13. overrun-local: Overrunning array of 12288 bytes at byte offset 12290 by dereferencing pointer "full_name + out_dir_length + 1". [Note: The source code implementation of the function has been overridden by a builtin model.] lib/pkg_editor/src/pkg_editor.c:1411:9: Type: Out-of-bounds read (OVERRUN) lib/pkg_editor/src/pkg_editor.c:1323:3: 1. path: Condition "!append_data(&info, 20UL /* sizeof (info) */, z_info, of, 0)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1330:3: 2. path: Condition "!append_data(dir_name, name_length, z_info, of, 0)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1385:5: 3. path: Condition "8192UL /* 2 * 4096 */ < name_length", taking false branch. lib/pkg_editor/src/pkg_editor.c:1385:5: 4. cond_at_most: Checking "8192UL < name_length" implies that "info.name_length" and "name_length" may be up to 8192 on the false branch. lib/pkg_editor/src/pkg_editor.c:1398:5: 5. path: Condition "dir == NULL", taking false branch. lib/pkg_editor/src/pkg_editor.c:1404:5: 6. path: Condition "entry", taking true branch. lib/pkg_editor/src/pkg_editor.c:1406:7: 7. path: Condition "strcmp(entry->d_name, ".") != 0", taking true branch. lib/pkg_editor/src/pkg_editor.c:1406:7: 8. path: Condition "strcmp(entry->d_name, "..") != 0", taking true branch. lib/pkg_editor/src/pkg_editor.c:1411:9: 9. overrun-local: Overrunning array of 8192 bytes at byte offset 8192 by dereferencing pointer "full_name + name_length". [Note: The source code implementation of the function has been overridden by a builtin model.]
1 parent 1755f43 commit 70fa51a

File tree

1 file changed

+31
-37
lines changed

1 file changed

+31
-37
lines changed

lib/pkg_editor/src/pkg_editor.c

Lines changed: 31 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,9 +1313,6 @@ static acl_pack_kind add_file(const char *out_file, FILE *of, const char *file,
13131313

13141314
static acl_pack_kind add_directory(const char *out_file, FILE *of,
13151315
const char *dir_name, ZInfo *z_info) {
1316-
#ifdef FULL_NAME_LENGTH
1317-
#undef FULL_NAME_LENGTH
1318-
#endif
13191316
acl_pkg_pack_info info;
13201317
size_t name_length = strlen(dir_name) + 1;
13211318

@@ -1341,24 +1338,31 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
13411338

13421339
// Now walk the directory processing each name.
13431340
{
1341+
13441342
#ifdef _WIN32
13451343
#define FULL_NAME_LENGTH (2 * MAX_PATH)
1344+
#else
1345+
#define FULL_NAME_LENGTH (2 * PATH_MAX)
1346+
#endif
1347+
13461348
char full_name[FULL_NAME_LENGTH];
1347-
if (FULL_NAME_LENGTH < name_length) {
1349+
1350+
// Full name must be large enough to store dir_name plus a trailing path
1351+
// separator
1352+
if (name_length + 1 > FULL_NAME_LENGTH) {
13481353
fprintf(stderr, "acl_pkg_pack: Failed to write to %s: %s\n", out_file,
13491354
"Directory name too long");
13501355
return PACK_END;
13511356
}
1357+
#ifdef _WIN32
13521358
HANDLE file_handle;
13531359
WIN32_FIND_DATA file_info;
13541360

13551361
// Partially initialize the full path name.
1356-
strncpy(full_name, dir_name, FULL_NAME_LENGTH);
1362+
strncpy(full_name, dir_name, FULL_NAME_LENGTH - 1);
13571363
strncpy(full_name + name_length - 1, "\\*.*",
1358-
FULL_NAME_LENGTH - name_length + 1);
1359-
if (full_name[FULL_NAME_LENGTH - 1] != '\0') {
1360-
full_name[FULL_NAME_LENGTH - 1] = '\0';
1361-
}
1364+
FULL_NAME_LENGTH - name_length);
1365+
full_name[FULL_NAME_LENGTH - 1] = '\0';
13621366

13631367
// Walk through all the files in the directory.
13641368
file_handle = FindFirstFile(full_name, &file_info);
@@ -1372,10 +1376,9 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
13721376

13731377
// Finish the full file name
13741378
strncpy(full_name + name_length, file_info.cFileName,
1375-
FULL_NAME_LENGTH - name_length);
1376-
if (full_name[FULL_NAME_LENGTH - 1] != '\0') {
1377-
full_name[FULL_NAME_LENGTH - 1] = '\0';
1378-
}
1379+
FULL_NAME_LENGTH - name_length - 1);
1380+
full_name[FULL_NAME_LENGTH - 1] = '\0';
1381+
13791382
if (add_file_or_dir(out_file, of, full_name, z_info) == PACK_END) {
13801383
FindClose(file_handle);
13811384
return PACK_END;
@@ -1386,14 +1389,6 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
13861389
// Linux
13871390
DIR *dir;
13881391
struct dirent *entry;
1389-
#define FULL_NAME_LENGTH (2 * PATH_MAX)
1390-
char full_name[FULL_NAME_LENGTH];
1391-
if (FULL_NAME_LENGTH < name_length) {
1392-
fprintf(stderr, "acl_pkg_pack: Failed to write to %s: %s\n", out_file,
1393-
"Directory name too long");
1394-
return PACK_END;
1395-
}
1396-
13971392
// Partially initialize the full path name.
13981393
strncpy(full_name, dir_name, FULL_NAME_LENGTH - 1);
13991394
full_name[FULL_NAME_LENGTH - 1] = '\0';
@@ -1414,10 +1409,9 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
14141409
// Finish the full file name, truncate name if the file is too long to
14151410
// avoid buffer overflow
14161411
size_t buffer_space_left = FULL_NAME_LENGTH - name_length;
1417-
strncpy(full_name + name_length, entry->d_name, buffer_space_left);
1418-
if (full_name[FULL_NAME_LENGTH - 1] != '\0') {
1419-
full_name[FULL_NAME_LENGTH - 1] = '\0';
1420-
}
1412+
strncpy(full_name + name_length, entry->d_name, buffer_space_left - 1);
1413+
full_name[FULL_NAME_LENGTH - 1] = '\0';
1414+
14211415
if (add_file_or_dir(out_file, of, full_name, z_info) == PACK_END) {
14221416
closedir(dir);
14231417
return PACK_END;
@@ -1427,6 +1421,7 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
14271421
}
14281422
closedir(dir);
14291423
#endif
1424+
#undef FULL_NAME_LENGTH
14301425
}
14311426
return PACK_DIR;
14321427
}
@@ -1561,12 +1556,6 @@ static int read_data(void *data, size_t size, ZInfo *z_info, FILE *in_fd) {
15611556
static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size,
15621557
FILE *input, const char *out_dir,
15631558
const char *routine_name) {
1564-
#ifdef FULL_NAME_LEN
1565-
#undef FULL_NAME_LEN
1566-
#endif
1567-
#ifdef NAME_LEN
1568-
#undef NAME_LEN
1569-
#endif
15701559
#ifdef _WIN32
15711560
#define FULL_NAME_LEN (3 * MAX_PATH)
15721561
#define NAME_LEN (2 * MAX_PATH)
@@ -1641,15 +1630,18 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size,
16411630
return 0;
16421631
}
16431632

1644-
// Generate the full name, truncate or zero pad to avoid buffer overflow
1645-
if (FULL_NAME_LEN < out_dir_length) {
1633+
// Generate the full name, truncate or zero pad to avoid buffer overflow.
1634+
// FULL_NAME_LEN must be large enough to store out_dir and a trailing path
1635+
// separator and null terminator
1636+
if (out_dir_length + 2 > FULL_NAME_LEN) {
16461637
fprintf(stderr, "%s: Directory name too long\n", routine_name);
1638+
inflateEnd(&z_info.strm);
1639+
return 0;
16471640
}
1641+
16481642
strncpy(full_name + out_dir_length + 1, name,
1649-
FULL_NAME_LEN - out_dir_length - 1);
1650-
if (full_name[FULL_NAME_LEN - 1] != '\0') {
1651-
full_name[FULL_NAME_LEN - 1] = '\0';
1652-
}
1643+
FULL_NAME_LEN - out_dir_length - 2);
1644+
full_name[FULL_NAME_LEN - 1] = '\0';
16531645

16541646
if (info.kind == PACK_DIR) {
16551647
#ifdef _WIN32
@@ -1718,6 +1710,8 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size,
17181710

17191711
inflateEnd(&z_info.strm);
17201712
return 1;
1713+
#undef FULL_NAME_LEN
1714+
#undef NAME_LEN
17211715
}
17221716

17231717
int acl_pkg_unpack_buffer(const char *buffer, size_t buffer_size,

0 commit comments

Comments
 (0)