Improvements - DevSkiller#17
Conversation
- len_bytes must now be an integer multiple of sizeof(struct sock_filter)
- Added overflow checks when assigning to prog->len (type unsigned short)
abouhasna
left a comment
abouhasna
left a comment
There was a problem hiding this comment.
Overall, this is a solid improvement over the original code, good catches on the overflow and alignment checks. There are just a few things I noticed which you can find in the comments.
|
|
||
| sc_apply_seccomp_filter(&prog_allow); | ||
| free(prog_allow.filter); | ||
| fclose(file); |
There was a problem hiding this comment.
If the file was already closed in previous functions, this may lead to undefined behavior
There was a problem hiding this comment.
file is only closed in previous functions when die() is called and the program exits. The control flow reaching here guarantees file to be open.
| struct sc_seccomp_file_header hdr = {}; | ||
|
|
||
| if (g_test_subprocess()) { | ||
| char *profile = NULL; |
There was a problem hiding this comment.
Resolved in ca73c34 for the happy test. The missing_header test does not have a free() for profile as the program dies during the test.
| char *profile = NULL; | ||
| int fd = 0; | ||
| make_seccomp_profile(&hdr, &fd, &profile); | ||
| FILE *file = sc_must_read_and_validate_header_from_file(profile, &hdr); |
There was a problem hiding this comment.
Resolved in ca73c34 for the happy test. The missing_header test does not have a fclose() for file as the program dies during the test.
3 participants
improvebranchdieis called.gitignorefor built filessc_must_read_filter_from_file()len_bytesmust now be an integer multiple ofsizeof(struct sock_filter)prog->len(type unsigned short)seccomp.hheader guard