Skip to content

Commit 56ab605

Browse files
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 d2fcc31 commit 56ab605

File tree

1 file changed

+28
-31
lines changed

1 file changed

+28
-31
lines changed

lib/pkg_editor/src/pkg_editor.c

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

13121312
static acl_pack_kind add_directory(const char *out_file, FILE *of,
13131313
const char *dir_name, ZInfo *z_info) {
1314-
#ifdef FULL_NAME_LENGTH
1315-
#undef FULL_NAME_LENGTH
1316-
#endif
13171314
acl_pkg_pack_info info;
13181315
size_t name_length = strlen(dir_name) + 1;
13191316

@@ -1339,24 +1336,31 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
13391336

13401337
// Now walk the directory processing each name.
13411338
{
1339+
13421340
#ifdef _WIN32
13431341
#define FULL_NAME_LENGTH (2 * MAX_PATH)
1342+
#else
1343+
#define FULL_NAME_LENGTH (2 * PATH_MAX)
1344+
#endif
1345+
13441346
char full_name[FULL_NAME_LENGTH];
1345-
if (FULL_NAME_LENGTH < name_length) {
1347+
1348+
// Full name must be large enough to store dir_name plus a trailing path
1349+
// separator
1350+
if (name_length + 1 > FULL_NAME_LENGTH) {
13461351
fprintf(stderr, "acl_pkg_pack: Failed to write to %s: %s\n", out_file,
13471352
"Directory name too long");
13481353
return PACK_END;
13491354
}
1355+
#ifdef _WIN32
13501356
HANDLE file_handle;
13511357
WIN32_FIND_DATA file_info;
13521358

13531359
// Partially initialize the full path name.
1354-
strncpy(full_name, dir_name, FULL_NAME_LENGTH);
1360+
strncpy(full_name, dir_name, FULL_NAME_LENGTH - 1);
13551361
strncpy(full_name + name_length - 1, "\\*.*",
1356-
FULL_NAME_LENGTH - name_length + 1);
1357-
if (full_name[FULL_NAME_LENGTH - 1] != '\0') {
1358-
full_name[FULL_NAME_LENGTH - 1] = '\0';
1359-
}
1362+
FULL_NAME_LENGTH - name_length);
1363+
full_name[FULL_NAME_LENGTH - 1] = '\0';
13601364

13611365
// Walk through all the files in the directory.
13621366
file_handle = FindFirstFile(full_name, &file_info);
@@ -1370,10 +1374,9 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
13701374

13711375
// Finish the full file name
13721376
strncpy(full_name + name_length, file_info.cFileName,
1373-
FULL_NAME_LENGTH - name_length);
1374-
if (full_name[FULL_NAME_LENGTH - 1] != '\0') {
1375-
full_name[FULL_NAME_LENGTH - 1] = '\0';
1376-
}
1377+
FULL_NAME_LENGTH - name_length - 1);
1378+
full_name[FULL_NAME_LENGTH - 1] = '\0';
1379+
13771380
if (add_file_or_dir(out_file, of, full_name, z_info) == PACK_END) {
13781381
FindClose(file_handle);
13791382
return PACK_END;
@@ -1384,14 +1387,6 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
13841387
// Linux
13851388
DIR *dir;
13861389
struct dirent *entry;
1387-
#define FULL_NAME_LENGTH (2 * PATH_MAX)
1388-
char full_name[FULL_NAME_LENGTH];
1389-
if (FULL_NAME_LENGTH < name_length) {
1390-
fprintf(stderr, "acl_pkg_pack: Failed to write to %s: %s\n", out_file,
1391-
"Directory name too long");
1392-
return PACK_END;
1393-
}
1394-
13951390
// Partially initialize the full path name.
13961391
strncpy(full_name, dir_name, FULL_NAME_LENGTH - 1);
13971392
full_name[FULL_NAME_LENGTH - 1] = '\0';
@@ -1412,10 +1407,9 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
14121407
// Finish the full file name, truncate name if the file is too long to
14131408
// avoid buffer overflow
14141409
size_t buffer_space_left = FULL_NAME_LENGTH - name_length;
1415-
strncpy(full_name + name_length, entry->d_name, buffer_space_left);
1416-
if (full_name[FULL_NAME_LENGTH - 1] != '\0') {
1417-
full_name[FULL_NAME_LENGTH - 1] = '\0';
1418-
}
1410+
strncpy(full_name + name_length, entry->d_name, buffer_space_left - 1);
1411+
full_name[FULL_NAME_LENGTH - 1] = '\0';
1412+
14191413
if (add_file_or_dir(out_file, of, full_name, z_info) == PACK_END) {
14201414
closedir(dir);
14211415
return PACK_END;
@@ -1639,15 +1633,18 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size,
16391633
return 0;
16401634
}
16411635

1642-
// Generate the full name, truncate or zero pad to avoid buffer overflow
1643-
if (FULL_NAME_LEN < out_dir_length) {
1636+
// Generate the full name, truncate or zero pad to avoid buffer overflow.
1637+
// FULL_NAME_LEN must be large enough to store out_dir and a trailing path
1638+
// separator and null terminator
1639+
if (out_dir_length + 2 > FULL_NAME_LEN) {
16441640
fprintf(stderr, "%s: Directory name too long\n", routine_name);
1641+
inflateEnd(&z_info.strm);
1642+
return 0;
16451643
}
1644+
16461645
strncpy(full_name + out_dir_length + 1, name,
1647-
FULL_NAME_LEN - out_dir_length - 1);
1648-
if (full_name[FULL_NAME_LEN - 1] != '\0') {
1649-
full_name[FULL_NAME_LEN - 1] = '\0';
1650-
}
1646+
FULL_NAME_LEN - out_dir_length - 2);
1647+
full_name[FULL_NAME_LEN - 1] = '\0';
16511648

16521649
if (info.kind == PACK_DIR) {
16531650
#ifdef _WIN32

0 commit comments

Comments
 (0)