Skip to content

Commit fd320d5

Browse files
Fix Coverity issue OVERRUN
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.] 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.]
1 parent 8152566 commit fd320d5

File tree

1 file changed

+41
-41
lines changed

1 file changed

+41
-41
lines changed

lib/pkg_editor/src/pkg_editor.c

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ static char *read_file_into_buffer(struct acl_pkg_file *pkg,
499499
fclose(f);
500500
return NULL;
501501
}
502-
const size_t file_size = (size_t)ftell_return;
502+
file_size = (size_t)ftell_return;
503503
rewind(f);
504504

505505
// slurp the whole file into allocated buf
@@ -1315,6 +1315,11 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
13151315
const char *dir_name, ZInfo *z_info) {
13161316
#ifdef FULL_NAME_LENGTH
13171317
#undef FULL_NAME_LENGTH
1318+
#endif
1319+
#ifdef _WIN32
1320+
#define FULL_NAME_LENGTH (2 * MAX_PATH)
1321+
#else
1322+
#define FULL_NAME_LENGTH (2 * PATH_MAX)
13181323
#endif
13191324
acl_pkg_pack_info info;
13201325
size_t name_length = strlen(dir_name) + 1;
@@ -1341,24 +1346,24 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
13411346

13421347
// Now walk the directory processing each name.
13431348
{
1344-
#ifdef _WIN32
1345-
#define FULL_NAME_LENGTH (2 * MAX_PATH)
13461349
char full_name[FULL_NAME_LENGTH];
1347-
if (FULL_NAME_LENGTH < name_length) {
1350+
1351+
// Full name must be large enough to store dir_name plus a trailing path
1352+
// separator
1353+
if (name_length + 1 > FULL_NAME_LENGTH) {
13481354
fprintf(stderr, "acl_pkg_pack: Failed to write to %s: %s\n", out_file,
13491355
"Directory name too long");
13501356
return PACK_END;
13511357
}
1358+
#ifdef _WIN32
13521359
HANDLE file_handle;
13531360
WIN32_FIND_DATA file_info;
13541361

13551362
// Partially initialize the full path name.
13561363
strncpy(full_name, dir_name, FULL_NAME_LENGTH);
13571364
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-
}
1365+
FULL_NAME_LENGTH - name_length);
1366+
full_name[FULL_NAME_LENGTH - 1] = '\0';
13621367

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

13731378
// Finish the full file name
13741379
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-
}
1380+
FULL_NAME_LENGTH - name_length - 1);
1381+
full_name[FULL_NAME_LENGTH - 1] = '\0';
1382+
13791383
if (add_file_or_dir(out_file, of, full_name, z_info) == PACK_END) {
13801384
FindClose(file_handle);
13811385
return PACK_END;
@@ -1386,14 +1390,6 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
13861390
// Linux
13871391
DIR *dir;
13881392
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-
13971393
// Partially initialize the full path name.
13981394
strncpy(full_name, dir_name, FULL_NAME_LENGTH - 1);
13991395
full_name[FULL_NAME_LENGTH - 1] = '\0';
@@ -1414,10 +1410,9 @@ static acl_pack_kind add_directory(const char *out_file, FILE *of,
14141410
// Finish the full file name, truncate name if the file is too long to
14151411
// avoid buffer overflow
14161412
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-
}
1413+
strncpy(full_name + name_length, entry->d_name, buffer_space_left - 1);
1414+
full_name[FULL_NAME_LENGTH - 1] = '\0';
1415+
14211416
if (add_file_or_dir(out_file, of, full_name, z_info) == PACK_END) {
14221417
closedir(dir);
14231418
return PACK_END;
@@ -1518,6 +1513,17 @@ int acl_pkg_pack(const char *out_file, const char **input_files_dirs) {
15181513
return 1 /* success */;
15191514
}
15201515

1516+
static void create_dir(char *dir_name) {
1517+
// Create output directory. We can ignore the error output since it will
1518+
// only delay the failure to the first attempt of creating a file in the
1519+
// (not) newly created directory.
1520+
#ifdef _WIN32
1521+
(void)CreateDirectory(dir_name, NULL);
1522+
#else
1523+
(void)mkdir(dir_name, 0755);
1524+
#endif
1525+
}
1526+
15211527
static int read_data(void *data, size_t size, ZInfo *z_info, FILE *in_fd) {
15221528
// We want to fill 'data' with 'size' bytes.
15231529
z_info->strm.next_out = data;
@@ -1603,12 +1609,7 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size,
16031609
return 0;
16041610
}
16051611

1606-
// Create output directory (ignore any errors).
1607-
#ifdef _WIN32
1608-
CreateDirectory(full_name, NULL);
1609-
#else
1610-
mkdir(full_name, 0755);
1611-
#endif
1612+
create_dir(full_name);
16121613
full_name[out_dir_length] = '/';
16131614

16141615
// Process the file until we hit the PACK_END record (or finish the
@@ -1640,22 +1641,21 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size,
16401641
return 0;
16411642
}
16421643

1643-
// Generate the full name, truncate or zero pad to avoid buffer overflow
1644-
if (FULL_NAME_LEN < out_dir_length) {
1644+
// Generate the full name, truncate or zero pad to avoid buffer overflow.
1645+
// FULL_NAME_LEN must be large enough to store out_dir and a trailing path
1646+
// separator and null terminator
1647+
if (out_dir_length + 2 > FULL_NAME_LEN) {
16451648
fprintf(stderr, "%s: Directory name too long\n", routine_name);
1649+
zlib_inflateEnd(&z_info.strm);
1650+
return 0;
16461651
}
1652+
16471653
strncpy(full_name + out_dir_length + 1, name,
1648-
FULL_NAME_LEN - out_dir_length - 1);
1649-
if (full_name[FULL_NAME_LEN - 1] != '\0') {
1650-
full_name[FULL_NAME_LEN - 1] = '\0';
1651-
}
1654+
FULL_NAME_LEN - out_dir_length - 2);
1655+
full_name[FULL_NAME_LEN - 1] = '\0';
16521656

16531657
if (info.kind == PACK_DIR) {
1654-
#ifdef _WIN32
1655-
CreateDirectory(full_name, NULL);
1656-
#else
1657-
mkdir(full_name, 0755);
1658-
#endif
1658+
create_dir(full_name);
16591659
} else {
16601660
// Read file contents
16611661
FILE *out_file = fopen(full_name, "wb");

0 commit comments

Comments
 (0)