Skip to content

Commit

Permalink
chore: Be even more explicit about integer range bounds.
Browse files Browse the repository at this point in the history
For coverity, which continues to think we're overrunning buffers when
at this point it's easy to prove we're not. Here would be the corrected
coverity finding:

6. Condition packet_length <= 105 /* 1 + 32 * 2 + 24 + 16 */, taking false branch.
7. Condition packet_length > 1024, taking false branch.

Now packet_length must be > 105 and <= 1024.

12. Condition len1 == packet_length - (89 /* 1 + 32 * 2 + 24 */) - 16, taking true branch.

len1 must be > 0 (105 - 89 - 16) and <= 919 (1024 - 89 - 16).

14. decr: Decrementing len1. The value of len1 is now between 0 and 919 (inclusive).

This is where coverity goes wrong: it thinks len1 could be up to 2147483629.

15. buffer access should be OK. Coverity thinks it's not.
  • Loading branch information
iphydf committed Mar 8, 2022
1 parent d15d9fa commit 5bec881
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
Original file line number Diff line number Diff line change
@@ -1 +1 @@
d67f642c84a92d92c799aa1d0bb9e0c2727ee67e565eb88dfa7a49f7e163cb82 /usr/local/bin/tox-bootstrapd
ba9c06f1079837ac20376c25bbf1411ddb6d8f86cc5c44cf58d053969b192653 /usr/local/bin/tox-bootstrapd
3 changes: 3 additions & 0 deletions toxcore/DHT.c
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,9 @@ int handle_request(const uint8_t *self_public_key, const uint8_t *self_secret_ke
}

assert(len1 == packet_length - CRYPTO_SIZE - CRYPTO_MAC_SIZE);
// Because coverity can't figure out this equation:
assert(len1 <= MAX_CRYPTO_REQUEST_SIZE - CRYPTO_SIZE - CRYPTO_MAC_SIZE);

request_id[0] = temp[0];
--len1;
memcpy(data, temp + 1, len1);
Expand Down

0 comments on commit 5bec881

Please sign in to comment.