Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static Analysis Fixes #44

Merged
Changes from 1 commit
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
82a374b
Makefile: add cppcheck static analysis target similar to libstb-secvar's
erichte-ibm Jul 12, 2023
2262f6b
guest/generate: remove always true conditions as reported by cppcheck
erichte-ibm Jul 12, 2023
2923b33
guest/generate: fix incorrect allocation check
erichte-ibm Jul 12, 2023
69c9284
guest/generate: unsigned (size_t) variables cannot be less than zero
erichte-ibm Jul 12, 2023
f4a14f0
guest/read: change printf format specifier to %zu to match variable type
erichte-ibm Jul 12, 2023
540d15b
guest/valdiate: change printf format specific to %zu to match variabl…
erichte-ibm Jul 12, 2023
92238b2
guest/validate: actually make a forward declartion of rc useful
erichte-ibm Jul 12, 2023
dff2217
guest/validate: put variables only used in an #ifdef also behind the …
erichte-ibm Jul 12, 2023
e33725b
guest/read: reduce scope of loop-only variables
erichte-ibm Jul 12, 2023
f605eae
guest/verify: refactor get_pk_and_kek_from_update_var()
erichte-ibm Jul 12, 2023
3d80649
guest/verify: functions should not just be one giant conditional
erichte-ibm Jul 12, 2023
a30dbe7
guest/verify: reduce the scope of the loop index
erichte-ibm Jul 12, 2023
dffda50
guest/read: size_t variables literally can't be -1
erichte-ibm Jul 12, 2023
5789c41
guest/read: size_t = %zu
erichte-ibm Jul 12, 2023
652864b
guest/read: reduce scope of variables that are only used by WRITE_FUN…
erichte-ibm Jul 12, 2023
2f0b468
guest/verify: remove extraneous code after break in parse_options
erichte-ibm Jul 18, 2023
ee5f417
generic: remove unusued function get_leading_whitespace
erichte-ibm Jul 12, 2023
b0588e2
host/generate: fix invalid malloc return check
erichte-ibm Jul 12, 2023
cb12c58
backends/host: unsigned integers still can't be less than zero
erichte-ibm Jul 12, 2023
45081f4
host/read: reduce the scope of variables in printReadable
erichte-ibm Jul 12, 2023
c06d938
host/read: fix weird pointer increment in readTS
erichte-ibm Jul 12, 2023
fd03f0d
host/validate: reduce scope of variables
erichte-ibm Jul 18, 2023
109a579
host/validate: add a NULL check to the **x509 parameter of parseX509
erichte-ibm Jul 18, 2023
470ef5c
host/verify: remove assignment with no effect
erichte-ibm Jul 18, 2023
71ee02a
host/validate: change ts_ptr to const in timestamp_is_empty
erichte-ibm Jul 18, 2023
81758e8
host/verify: remove redundant check, as <= 1 implies !
erichte-ibm Jul 18, 2023
d702bd4
guest: use system endian.h, as it's entirely ambiguous which endian.h…
erichte-ibm Jul 18, 2023
1146beb
Makefile: fix include pathing for cppcheck
erichte-ibm Jul 18, 2023
d5cc405
guest/generate: fix format string typing (int -> uint)
erichte-ibm Jul 18, 2023
6c7366a
guest/read: fix format string typing
erichte-ibm Jul 18, 2023
3b1ba71
guest/validate: remove needless esl_size variable alias from validate…
erichte-ibm Jul 18, 2023
b606dee
guest/validate: fix format string typing
erichte-ibm Jul 18, 2023
161591c
guest/validate: unsigned integers still can't be less than zero
erichte-ibm Jul 18, 2023
16e257b
guest/verify: remove redundant check, unsigned less than 0, etc etc
erichte-ibm Jul 18, 2023
0ccabc9
more unsigned printf fixes
erichte-ibm Jul 19, 2023
c96d802
host/validate: %zu
erichte-ibm Jul 19, 2023
947ab14
host/validate: remove useless eslsize alias
erichte-ibm Jul 19, 2023
4cf8030
host/validate: always free a buffer that always exists
erichte-ibm Jul 19, 2023
391cd0d
host/validate: unsigned still can't be less than zero
erichte-ibm Jul 19, 2023
d2c4ed2
guest/read: remove conditional that is always true in read_auth
erichte-ibm Jul 19, 2023
5518ca2
host/read: standard for unsigned unchanged, still cannot be less than…
erichte-ibm Jul 19, 2023
0617f05
host/generate: check that ESL is not NULL is getPreHashForSecVar
erichte-ibm Jul 19, 2023
49cbda7
guest/read: check auth_data is not NULL in read_auth
erichte-ibm Jul 19, 2023
8f6703e
host/verify: fix potential memory leak in setupBanks
erichte-ibm Jul 19, 2023
16a6f32
guest/verify: fix potential double free in get_pk_and_kek from update…
erichte-ibm Jul 19, 2023
b0da6ee
host/generate: fix unsigned printf format strings with CRYPTO_WRITE_F…
erichte-ibm Jul 20, 2023
9878d6f
Makefile/cppcheck: suppress false positives, reorganize cppcheck flag…
erichte-ibm Jul 20, 2023
1e4202d
guest/verify: display usage if arguments fail to validate
erichte-ibm Aug 1, 2023
4af5d99
guest/validate: reduce scope of rc in validate_esl
erichte-ibm Aug 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions backends/guest/common/read.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,17 @@ int print_variables(const uint8_t *buffer, size_t buffer_size, const uint8_t *va
{
int rc;
ssize_t esl_data_size = buffer_size, cert_size;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, we should split this into smaller functions.

But if you are going to reduce scope of variables then you might as well go all the way:

  • data_size is useless and can be removed and replaced with cert_size
  • cert_size, esd can be declared in the second loop
  • signature_type, sig_list can go in first loop
  • I don't think offset is being used properly in this function at all, in the first loop we use it for offset to the next ESL header, in the second, loop we use it as an offset to the ESL data. I doubt this works if there are ever two appended ESL structures (which I believe is possible with dbx, unless something changed)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this was mostly to silence cppcheck warnings, most of the rest of the scope reductions will probably be removed with a function rewrite. Skipping for now.

size_t esl_size = 0, next_esl_size = 0, data_size = 0;
size_t sig_offset = 0, signature_size = 0, count = 0, offset = 0;
size_t count = 0, offset = 0;
uint8_t *cert = NULL, *signature_type = NULL, *esl_data = (uint8_t *)buffer;
sv_esl_t *sig_list;
sv_esd_t *esd = NULL;
crypto_x509_t *x509 = NULL;

while (esl_data_size > 0) {
// TODO: consider breaking this down into functions, to avoid these scope reductions
size_t esl_size, next_esl_size, sig_offset;
size_t signature_size;

if (esl_data_size < sizeof(sv_esl_t)) {
prlog(PR_ERR,
"ERROR: ESL has %zd bytes and is smaller than an ESL (%zd bytes),"
Expand Down Expand Up @@ -159,6 +162,8 @@ int print_variables(const uint8_t *buffer, size_t buffer_size, const uint8_t *va

/* reads the esd from esl */
while (esl_size > 0) {
size_t data_size;

esd = (sv_esd_t *)(esl_data + (offset + sig_offset));
data_size = signature_size - sizeof(sv_esd_t);
cert = esd->signature_data;
Expand Down