Skip to content

Comments

Only write to pdays/psecs if they are not null#29337

Closed
nhorman wants to merge 1 commit intoopenssl:masterfrom
nhorman:fix_gmtime_diff
Closed

Only write to pdays/psecs if they are not null#29337
nhorman wants to merge 1 commit intoopenssl:masterfrom
nhorman:fix_gmtime_diff

Conversation

@nhorman
Copy link
Contributor

@nhorman nhorman commented Dec 8, 2025

We have a few cases in which one of the paramters passed to ASN1_TIME_diff is null (i.e. the caller doesn't care about the psec differnce and so passes NULL as that pointer parameter).

However, OPENSSL_gmtime_diff assumes both pointers are valid, and so writes to them unilaterally resulting in a crash as observed here: #29333 (comment)

Check the pointers before writing to them.

We have a few cases in which one of the paramters passed to
ASN1_TIME_diff is null (i.e. the caller doesn't care about the psec
differnce and so passes NULL as that pointer parameter).

However, OPENSSL_gmtime_diff assumes both pointers are valid, and so
writes to them unilaterally resulting in a crash as observed here:
openssl#29333 (comment)

Check the pointers before writing to them.
Copy link
Contributor

@jogme jogme left a comment

Choose a reason for hiding this comment

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

makes sense, good catch!

@t8m t8m added approval: done This pull request has the required number of approvals triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing and removed approval: review pending This pull request needs review by a committer labels Dec 8, 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 Dec 9, 2025
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@nhorman
Copy link
Contributor Author

nhorman commented Dec 9, 2025

merged to master, thank you!

@nhorman nhorman closed this Dec 9, 2025
openssl-machine pushed a commit that referenced this pull request Dec 9, 2025
We have a few cases in which one of the paramters passed to
ASN1_TIME_diff is null (i.e. the caller doesn't care about the psec
differnce and so passes NULL as that pointer parameter).

However, OPENSSL_gmtime_diff assumes both pointers are valid, and so
writes to them unilaterally resulting in a crash as observed here:
#29333 (comment)

Check the pointers before writing to them.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
(Merged from #29337)
cxx194832 pushed a commit to cxx194832/openssl that referenced this pull request Dec 12, 2025
We have a few cases in which one of the paramters passed to
ASN1_TIME_diff is null (i.e. the caller doesn't care about the psec
differnce and so passes NULL as that pointer parameter).

However, OPENSSL_gmtime_diff assumes both pointers are valid, and so
writes to them unilaterally resulting in a crash as observed here:
openssl#29333 (comment)

Check the pointers before writing to them.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
(Merged from openssl#29337)
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 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.

7 participants