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

Add further sanity checking of hdr->error_no #805

Merged
merged 2 commits into from
Aug 19, 2024
Merged

Add further sanity checking of hdr->error_no #805

merged 2 commits into from
Aug 19, 2024

Conversation

jlevon
Copy link
Collaborator

@jlevon jlevon commented Aug 16, 2024

CID 467267:  Insecure data handling  (INTEGER_OVERFLOW)
The cast of "hdr->error_no" to a signed type could result in a negative number.

Indeed, if a client sends a very large ->error_no, this could end up
with a negative errno value. This doesn't seem like an issue, but
nonetheless tighten up our validation.

For some reason Coverity only complained about tran_pipe.c, but the same
problem exists in tran_sock.c.

Signed-off-by: John Levon [email protected]

jlevon added 2 commits August 16, 2024 17:57
>>>     CID 467268:    (INTEGER_OVERFLOW)
>>>     Expression "32UL + bitmap_size", which is equal to 31, where
"bitmap_size" is known to be equal to 18446744073709551615, overflows
the type that receives it, an unsigned integer 64 bits wide.
824         size_t size = sizeof(*res) + sizeof(*report) + bitmap_size;

It's correct, this could overflow, though as this is example code, it
doesn't matter. Nonetheless let's make this be a saturating add.

Signed-off-by: John Levon <[email protected]>
>>>     CID 467267:  Insecure data handling  (INTEGER_OVERFLOW)
>>>     The cast of "hdr->error_no" to a signed type could result in a negative number.

Indeed, if a client sends a very large ->error_no, this could end up
with a negative errno value. This doesn't seem like an issue, but
nonetheless tighten up our validation.

For some reason Coverity only complained about tran_pipe.c, but the same
problem exists in tran_sock.c.

Signed-off-by: John Levon <[email protected]>
@jlevon jlevon requested review from tmakatos and swapnili August 16, 2024 17:09
Base automatically changed from coverity to master August 19, 2024 10:43
@jlevon jlevon merged commit b1a156d into master Aug 19, 2024
7 checks passed
@jlevon jlevon deleted the coverity2 branch August 19, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants