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

OPENSSL_hexstr2buf_ex(): Handle zero-length input correctly #24770

Closed
wants to merge 1 commit into from

Conversation

t8m
Copy link
Member

@t8m t8m commented Jul 1, 2024

In case of zero-length input the code wrote one byte before the start of the output buffer. The length
of the output was also reported incorrectly in this case.

@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending triaged: bug The issue/pr is/fixes a bug severity: important Important bugs affecting a released version branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 tests: present The PR has suitable tests present branch: 3.2 Merge to openssl-3.2 branch: 3.3 Merge to openssl-3.3 labels Jul 1, 2024
In case of zero-length input the code wrote one byte
before the start of the output buffer. The length
of the output was also reported incorrectly in this case.
@t8m t8m force-pushed the print-attr-overflow branch from 03cd9b2 to 026f291 Compare July 1, 2024 07:35
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Jul 1, 2024
@kroeckx
Copy link
Member

kroeckx commented Jul 1, 2024

Shouldn't we just return an error on a zero length input?

@t8m t8m linked an issue Jul 1, 2024 that may be closed by this pull request
@t8m
Copy link
Member Author

t8m commented Jul 1, 2024

Shouldn't we just return an error on a zero length input?

Why? Also OPENSSL_buf2hexstr() below returns an empty string in such case.

@t8m t8m 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 Jul 1, 2024
@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 2, 2024
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@t8m
Copy link
Member Author

t8m commented Jul 2, 2024

Merged to all the active branches. Thank you for the reviews.

@t8m t8m closed this Jul 2, 2024
openssl-machine pushed a commit that referenced this pull request Jul 2, 2024
In case of zero-length input the code wrote one byte
before the start of the output buffer. The length
of the output was also reported incorrectly in this case.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from #24770)

(cherry picked from commit 3f7b355)
openssl-machine pushed a commit that referenced this pull request Jul 2, 2024
In case of zero-length input the code wrote one byte
before the start of the output buffer. The length
of the output was also reported incorrectly in this case.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from #24770)

(cherry picked from commit 3f7b355)
openssl-machine pushed a commit that referenced this pull request Jul 2, 2024
In case of zero-length input the code wrote one byte
before the start of the output buffer. The length
of the output was also reported incorrectly in this case.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from #24770)
openssl-machine pushed a commit that referenced this pull request Jul 2, 2024
In case of zero-length input the code wrote one byte
before the start of the output buffer. The length
of the output was also reported incorrectly in this case.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from #24770)

(cherry picked from commit 3f7b355)
openssl-machine pushed a commit that referenced this pull request Jul 2, 2024
In case of zero-length input the code wrote one byte
before the start of the output buffer. The length
of the output was also reported incorrectly in this case.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from #24770)

(cherry picked from commit 3f7b355)
eclipse-oniro-oh-bot pushed a commit to eclipse-oniro-mirrors/third_party_openssl that referenced this pull request Jul 9, 2024
In case of zero-length input the code wrote one byte
before the start of the output buffer. The length
of the output was also reported incorrectly in this case.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from openssl/openssl#24770)

(cherry picked from commit 3f7b355733407cf777bfad5ce5b79610588bacc5)
Signed-off-by: hhhFun <[email protected]>
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 branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 branch: 3.2 Merge to openssl-3.2 branch: 3.3 Merge to openssl-3.3 severity: fips change The pull request changes FIPS provider sources severity: important Important bugs affecting a released version tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate Clusterfuzz heap buffer overflow
6 participants