Skip to content

Conversation

nhorman
Copy link
Contributor

@nhorman nhorman commented Jul 26, 2025

A few coverity reports came in over the weekend, this PR addresses:

https://scan5.scan.coverity.com/#/project-view/60762/10222?selectedIssue=1659227
https://scan5.scan.coverity.com/#/project-view/60762/10222?selectedIssue=1659225
https://scan5.scan.coverity.com/#/project-view/60762/10222?selectedIssue=1659222

All amount to improper NULL checks in various recently modified locations in s_server.c

nhorman added 3 commits July 26, 2025 09:18
We assign an allocated pointer to *sk_resp but only check if sk_resp is
NULL when sk_resp is a pointer to a pointer

Addresses https://scan5.scan.coverity.com/#/project-view/60762/10222?selectedIssue=1659227
Its possible for get_ocsp_resp_from_responder to return OK after having
freed *sk_resp without setting the freed pointer to NULL, leading us to
set a garbage pointer in other code.

Ensure that we set it to NULL after freeing

Addresses https://scan5.scan.coverity.com/#/project-view/60762/10222?selectedIssue=1659225
We assign an allocation to *sk_resp, but only check for NULL on sk_resp,
not the value it points to.

Addresses https://scan5.scan.coverity.com/#/project-view/60762/10222?selectedIssue=1659222
@nhorman nhorman self-assigned this Jul 26, 2025
@nhorman nhorman added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels Jul 26, 2025
@nhorman nhorman requested review from Sashan, DDvO, mattcaswell and jogme July 26, 2025 13:37
@nhorman nhorman moved this to Waiting Review in Development Board Jul 26, 2025
@github-project-automation github-project-automation bot moved this from Waiting Review to Waiting Merge in Development Board Jul 28, 2025
@DDvO DDvO added approval: done This pull request has the required number of approvals triaged: bug The issue/pr is/fixes a bug and removed approval: review pending This pull request needs review by a committer labels Jul 28, 2025
@t8m t8m added the tests: exempted The PR is exempt from requirements for testing label Jul 28, 2025
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Jul 29, 2025
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@nhorman
Copy link
Contributor Author

nhorman commented Jul 29, 2025

merged to master, thank you

@nhorman nhorman closed this Jul 29, 2025
@github-project-automation github-project-automation bot moved this from Waiting Merge to Done in Development Board Jul 29, 2025
openssl-machine pushed a commit that referenced this pull request Jul 29, 2025
We assign an allocated pointer to *sk_resp but only check if sk_resp is
NULL when sk_resp is a pointer to a pointer

Addresses https://scan5.scan.coverity.com/#/project-view/60762/10222?selectedIssue=1659227

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #28101)
openssl-machine pushed a commit that referenced this pull request Jul 29, 2025
Its possible for get_ocsp_resp_from_responder to return OK after having
freed *sk_resp without setting the freed pointer to NULL, leading us to
set a garbage pointer in other code.

Ensure that we set it to NULL after freeing

Addresses https://scan5.scan.coverity.com/#/project-view/60762/10222?selectedIssue=1659225

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #28101)
openssl-machine pushed a commit that referenced this pull request Jul 29, 2025
We assign an allocation to *sk_resp, but only check for NULL on sk_resp,
not the value it points to.

Addresses https://scan5.scan.coverity.com/#/project-view/60762/10222?selectedIssue=1659222

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #28101)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants