Skip to content

Conversation

@ndossche
Copy link
Contributor

@ndossche ndossche commented Jan 30, 2023

default_check() can return a zero value to indicate an internal error in one condition for the PRE_CTRL_STR_TO_PARAMS state. This state can be reached from the default_fixup_args() function which does not check for a zero value. All other callers of default_check() in that file do check for a zero return value. Fix it by changing the check to <= 0.

CLA: trivial

Please note that I found this using a static analysis tool I am developing at the moment. It could therefore be a false positive bug. I manually reviewed the case to be extra sure that it is a real bug.

Checklist
  • documentation is added or updated
  • tests are added or updated

default_check() can return a zero value to indicate an internal error in
one condition for the PRE_CTRL_STR_TO_PARAMS state. This state can be
reached from the default_fixup_args() function which does not check for
a zero value. All other callers of default_check() in that file do check
for a zero return value. Fix it by changing the check to <= 0.

CLA: trivial
@t8m
Copy link
Member

t8m commented Jan 30, 2023

OK with CLA: trivial

@t8m t8m added branch: master Applies to master branch approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug cla: trivial One of the commits is marked as 'CLA: trivial' branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) tests: exempted The PR is exempt from requirements for testing labels Jan 30, 2023
Copy link
Member

@hlandau hlandau left a comment

Choose a reason for hiding this comment

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

OK with trivial.

@hlandau hlandau added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Feb 2, 2023
@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

@paulidale paulidale 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 Feb 5, 2023
@t8m
Copy link
Member

t8m commented Feb 8, 2023

Merged to master, 3.1, and 3.0 branches. Thank you for your contribution.

@t8m t8m closed this Feb 8, 2023
openssl-machine pushed a commit that referenced this pull request Feb 8, 2023
default_check() can return a zero value to indicate an internal error in
one condition for the PRE_CTRL_STR_TO_PARAMS state. This state can be
reached from the default_fixup_args() function which does not check for
a zero value. All other callers of default_check() in that file do check
for a zero return value. Fix it by changing the check to <= 0.

CLA: trivial

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #20175)

(cherry picked from commit 650f047)
openssl-machine pushed a commit that referenced this pull request Feb 8, 2023
default_check() can return a zero value to indicate an internal error in
one condition for the PRE_CTRL_STR_TO_PARAMS state. This state can be
reached from the default_fixup_args() function which does not check for
a zero value. All other callers of default_check() in that file do check
for a zero return value. Fix it by changing the check to <= 0.

CLA: trivial

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #20175)

(cherry picked from commit 650f047)
openssl-machine pushed a commit that referenced this pull request Feb 8, 2023
default_check() can return a zero value to indicate an internal error in
one condition for the PRE_CTRL_STR_TO_PARAMS state. This state can be
reached from the default_fixup_args() function which does not check for
a zero value. All other callers of default_check() in that file do check
for a zero return value. Fix it by changing the check to <= 0.

CLA: trivial

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #20175)
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 Applies to master branch branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) cla: trivial One of the commits is marked as 'CLA: trivial' tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants