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.

Sign up for GitHub

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

Jump to bottom

Return value confusion in cms_sd_asn1_ctrl() #21986

Closed
botovq opened this issue Sep 6, 2023 · 1 comment
Closed

Return value confusion in cms_sd_asn1_ctrl() #21986

botovq opened this issue Sep 6, 2023 · 1 comment
Labels
branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 triaged: bug The issue/pr is/fixes a bug

Comments

@botovq
Copy link
Contributor

botovq commented Sep 6, 2023

In cms_sd.c, the function cms_sd_asn1_sign() is error checked as if it had a Boolean return value. However, since #13088, it can also return -1.
@botovq botovq added the issue: bug report The issue was opened to report a bug label Sep 6, 2023
@paulidale paulidale added triaged: bug The issue/pr is/fixes a bug branch: master Merge to master branch help wanted branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 and removed issue: bug report The issue was opened to report a bug labels Sep 6, 2023
@paulidale
Copy link
Contributor

The function changed in that PR that can return -1 appears to be cms_sd_asn1_ctrl().
It's return code is checked via !cms_sd_asn1_ctrl() and should be cms_sd_asn1_ctrl() <= 0.

@paulidale paulidale added tests: exempted The PR is exempt from requirements for testing and removed help wanted labels Sep 6, 2023
paulidale added a commit to paulidale/openssl that referenced this issue Sep 6, 2023
@paulidale
Check error return from cms_sd_asn1_ctrl() correctly.
25b7025 
Fixes openssl#21986
@paulidale paulidale mentioned this issue Sep 6, 2023
Closed
2 tasks
@botovq botovq changed the title Return value confusion in cms_sd_asn1_sign() Return value confusion in cms_sd_asn1_ctrl() Sep 7, 2023
paulidale added a commit to paulidale/openssl that referenced this issue Sep 7, 2023
@paulidale
Check error return from cms_sd_asn1_ctrl() correctly.
2509ad3 
Fixes openssl#21986
paulidale added a commit to paulidale/openssl that referenced this issue Sep 7, 2023
@paulidale
Add test case for openssl#21986
cca368b
paulidale added a commit to paulidale/openssl that referenced this issue Sep 7, 2023
@paulidale
Add test case for openssl#21986
5bcec7d
@t8m t8m removed the tests: exempted The PR is exempt from requirements for testing label Sep 7, 2023
openssl-machine pushed a commit that referenced this issue Sep 8, 2023
@paulidale
Add test case for #21986
c870a46 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #21988)
openssl-machine pushed a commit that referenced this issue Sep 8, 2023
@paulidale
Check error return from cms_sd_asn1_ctrl() correctly.
f04c64b 
Fixes #21986

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #21988)

(cherry picked from commit 00a413e)
openssl-machine pushed a commit that referenced this issue Sep 8, 2023
@paulidale
Add test case for #21986
1b40dbf 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #21988)

(cherry picked from commit c870a46)
openssl-machine pushed a commit that referenced this issue Sep 8, 2023
@paulidale
Check error return from cms_sd_asn1_ctrl() correctly.
2482061 
Fixes #21986

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #21988)

(cherry picked from commit 00a413e)
openssl-machine pushed a commit that referenced this issue Sep 8, 2023
@paulidale
Add test case for #21986
41136a9 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #21988)

(cherry picked from commit c870a46)
xl32 pushed a commit to xl32/openssl that referenced this issue Sep 29, 2023
@paulidale @xl32
Check error return from cms_sd_asn1_ctrl() correctly.
0f0ad5b 
Fixes openssl#21986

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from openssl#21988)

(cherry picked from commit 00a413e)
xl32 pushed a commit to xl32/openssl that referenced this issue Sep 29, 2023
@paulidale @xl32
Add test case for openssl#21986
cf262c0 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from openssl#21988)

(cherry picked from commit c870a46)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Check error return from cms_sd_asn1_ctrl() correctly. paulidale/openssl
3 participants
@t8m @botovq @paulidale