Skip to content

Commit 2477203

Browse files
IlanTruanovskyIlanTruanovsky
committed
Fixes A Coverity TAINTED_SCALAR issue regarding an unbounded malloc
Fixes: ``` lib/pkg_editor/src/pkg_editor.c:1681:11: Type: Untrusted allocation size (TAINTED_SCALAR) lib/pkg_editor/src/pkg_editor.c:1591:3: Tainted data flows to a taint sink 1. path: Condition "buffer != NULL", taking false branch. lib/pkg_editor/src/pkg_editor.c:1596:5: 2. path: Condition "input != NULL", taking true branch. lib/pkg_editor/src/pkg_editor.c:1596:5: 3. path: Falling through to end of if statement. lib/pkg_editor/src/pkg_editor.c:1601:3: 4. path: Condition "ret != 0", taking false branch. lib/pkg_editor/src/pkg_editor.c:1612:3: 5. path: Condition "z_info.strm.avail_in > 0", taking false branch. lib/pkg_editor/src/pkg_editor.c:1612:3: 6. path: Condition "input != NULL", taking true branch. lib/pkg_editor/src/pkg_editor.c:1612:3: 7. path: Condition "!feof(input)", taking true branch. lib/pkg_editor/src/pkg_editor.c:1614:5: 8. path: Condition "!read_data(&info, 20UL /* sizeof (info) */, &z_info, input)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1619:5: 9. path: Condition "info.magic != 3203399403U", taking false branch. lib/pkg_editor/src/pkg_editor.c:1627:5: 10. path: Condition "info.kind == PACK_END", taking false branch. lib/pkg_editor/src/pkg_editor.c:1632:5: 11. path: Condition "!read_data(name, info.name_length, &z_info, input)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1642:5: 12. path: Condition "out_dir_length + 2 > 12288UL /* 3 * 4096 */", taking false branch. lib/pkg_editor/src/pkg_editor.c:1652:5: 13. path: Condition "info.kind == PACK_DIR", taking true branch. lib/pkg_editor/src/pkg_editor.c:1654:5: 14. path: Falling through to end of if statement. lib/pkg_editor/src/pkg_editor.c:1711:3: 15. path: Jumping back to the beginning of the loop. lib/pkg_editor/src/pkg_editor.c:1612:3: 16. path: Condition "z_info.strm.avail_in > 0", taking true branch. lib/pkg_editor/src/pkg_editor.c:1614:5: 17. path: Condition "!read_data(&info, 20UL /* sizeof (info) */, &z_info, input)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1619:5: 18. path: Condition "info.magic != 3203399403U", taking false branch. lib/pkg_editor/src/pkg_editor.c:1627:5: 19. path: Condition "info.kind == PACK_END", taking false branch. lib/pkg_editor/src/pkg_editor.c:1632:5: 20. path: Condition "!read_data(name, info.name_length, &z_info, input)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1642:5: 21. path: Condition "out_dir_length + 2 > 12288UL /* 3 * 4096 */", taking false branch. lib/pkg_editor/src/pkg_editor.c:1652:5: 22. path: Condition "info.kind == PACK_DIR", taking true branch. lib/pkg_editor/src/pkg_editor.c:1654:5: 23. path: Falling through to end of if statement. lib/pkg_editor/src/pkg_editor.c:1711:3: 24. path: Jumping back to the beginning of the loop. lib/pkg_editor/src/pkg_editor.c:1612:3: 25. path: Condition "z_info.strm.avail_in > 0", taking true branch. lib/pkg_editor/src/pkg_editor.c:1614:5: 26. tainted_argument: Calling function "read_data" taints argument "info". lib/pkg_editor/src/pkg_editor.c:1530:3: Tainted data flows to a taint sink 26.1. var_assign_parm: Assigning: "z_info->strm.next_out" = "data". lib/pkg_editor/src/pkg_editor.c:1534:5: 26.2. path: Condition "z_info->strm.avail_in == 0", taking true branch. lib/pkg_editor/src/pkg_editor.c:1537:7: 26.3. path: Condition "in_fd == NULL", taking false branch. lib/pkg_editor/src/pkg_editor.c:1537:7: 26.4. path: Condition "feof(in_fd)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1541:7: 26.5. tainted_data_argument: Calling function "fread" taints parameter "*z_info->buffer". [Note: The source code implementation of the function has been overridden by a builtin model.] lib/pkg_editor/src/pkg_editor.c:1542:7: 26.6. path: Condition "count < 1", taking false branch. lib/pkg_editor/src/pkg_editor.c:1547:7: 26.7. var_assign_alias: Assigning: "z_info->strm.next_in" = "z_info->buffer", which taints "z_info->strm.next_in". lib/pkg_editor/src/pkg_editor.c:1550:5: 26.8. tainted_data_transitive: Calling function "inflate" with tainted argument "*z_info->strm.next_in" taints "*z_info->strm.next_out". lib/pkg_editor/src/pkg_editor.c:1551:5: 26.9. path: Condition "ret != -2", taking true branch. lib/pkg_editor/src/pkg_editor.c:1551:5: 26.10. path: Falling through to end of if statement. lib/pkg_editor/src/pkg_editor.c:1552:5: 26.11. path: Condition "ret == 1", taking true branch. lib/pkg_editor/src/pkg_editor.c:1554:7: 26.12. path: Condition "z_info->strm.avail_out == 0", taking false branch. lib/pkg_editor/src/pkg_editor.c:1614:5: 27. path: Condition "!read_data(&info, 20UL /* sizeof (info) */, &z_info, input)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1619:5: 28. path: Condition "info.magic != 3203399403U", taking false branch. lib/pkg_editor/src/pkg_editor.c:1627:5: 29. path: Condition "info.kind == PACK_END", taking false branch. lib/pkg_editor/src/pkg_editor.c:1632:5: 30. path: Condition "!read_data(name, info.name_length, &z_info, input)", taking false branch. lib/pkg_editor/src/pkg_editor.c:1642:5: 31. path: Condition "out_dir_length + 2 > 12288UL /* 3 * 4096 */", taking false branch. lib/pkg_editor/src/pkg_editor.c:1652:5: 32. path: Condition "info.kind == PACK_DIR", taking false branch. lib/pkg_editor/src/pkg_editor.c:1657:7: 33. path: Condition "out_file == NULL", taking false branch. lib/pkg_editor/src/pkg_editor.c:1663:7: 34. path: Condition "info.file_length > 0", taking true branch. lib/pkg_editor/src/pkg_editor.c:1663:7: 35. lower_bounds: Checking lower bounds of unsigned scalar "info.file_length" by taking the true branch of "info.file_length > 0U". lib/pkg_editor/src/pkg_editor.c:1665:9: 36. path: Condition "info.file_length < 65536UL /* sizeof (buf) */", taking false branch. lib/pkg_editor/src/pkg_editor.c:1665:9: 37. lower_bounds: Checking lower bounds of unsigned scalar "info.file_length" by taking the false branch of "info.file_length < 65536UL". lib/pkg_editor/src/pkg_editor.c:1681:11: 38. tainted_data: Passing tainted expression "info.file_length" to "malloc", which uses it as an allocation size. lib/pkg_editor/src/pkg_editor.c:1681:11: 39. remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range. ```
1 parent 70beec0 commit 2477203

File tree

1 file changed

+22
-31
lines changed

1 file changed

+22
-31
lines changed

‎lib/pkg_editor/src/pkg_editor.c

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,50 +1668,41 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size,
16681668
inflateEnd(&z_info.strm);
16691669
return 0;
16701670
}
1671-
if (info.file_length > 0) {
1672-
char buf[64 * 1024];
1673-
if (info.file_length < sizeof(buf)) {
1674-
if (!read_data(buf, info.file_length, &z_info, input)) {
1671+
fclose(out_file);
1672+
out_file = fopen(full_name, "ab");
1673+
if (out_file == NULL) {
1674+
fprintf(stderr, "%s: Unable to open %s for appending: %s\n",
1675+
routine_name, full_name, strerror(errno));
1676+
inflateEnd(&z_info.strm);
1677+
return 0;
1678+
}
1679+
char buf[64 * 1024];
1680+
size_t left_to_read = info.file_length;
1681+
// If left_to_read == 0 then read_data() will error out with Z_BUF_ERROR
1682+
// since we'd be giving it '0' bytes of output space to put the
1683+
// decompressed data
1684+
if (left_to_read > 0) {
1685+
for (;;) {
1686+
const size_t num_to_read =
1687+
(left_to_read > sizeof(buf)) ? sizeof(buf) : left_to_read;
1688+
if (!read_data(buf, num_to_read, &z_info, input)) {
16751689
fprintf(stderr, "%s: Error reading file data for %s from buffer\n",
16761690
routine_name, full_name);
16771691
fclose(out_file);
16781692
inflateEnd(&z_info.strm);
16791693
return 0;
16801694
}
1681-
if (fwrite(buf, info.file_length, 1, out_file) != 1) {
1695+
if (fwrite(buf, num_to_read, 1, out_file) != 1) {
16821696
fprintf(stderr, "%s: Failed to write to %s: %s\n", routine_name,
16831697
full_name, strerror(errno));
16841698
fclose(out_file);
16851699
inflateEnd(&z_info.strm);
16861700
return 0;
16871701
}
1688-
} else {
1689-
char *buf2 = malloc(info.file_length);
1690-
if (buf2 == NULL) {
1691-
fprintf(stderr, "%s: Failed to allocate buffer to write %s: %s\n",
1692-
routine_name, full_name, strerror(errno));
1693-
fclose(out_file);
1694-
free(buf2);
1695-
inflateEnd(&z_info.strm);
1696-
return PACK_END;
1697-
}
1698-
if (!read_data(buf2, info.file_length, &z_info, input)) {
1699-
fprintf(stderr, "%s: Error reading file data for %s from buffer\n",
1700-
routine_name, full_name);
1701-
fclose(out_file);
1702-
free(buf2);
1703-
inflateEnd(&z_info.strm);
1704-
return 0;
1705-
}
1706-
if (fwrite(buf2, info.file_length, 1, out_file) != 1) {
1707-
fprintf(stderr, "%s: Failed to write to %s: %s\n", routine_name,
1708-
full_name, strerror(errno));
1709-
fclose(out_file);
1710-
free(buf2);
1711-
inflateEnd(&z_info.strm);
1712-
return 0;
1702+
if (left_to_read <= sizeof(buf)) {
1703+
break;
17131704
}
1714-
free(buf2);
1705+
left_to_read -= sizeof(buf);
17151706
}
17161707
}
17171708
fclose(out_file);

0 commit comments

Comments
 (0)