Skip to content

Adds multiple checks to avoid buffer over reads [1.1.0]#5686

Closed
catenacyber wants to merge 1 commit intoopenssl:OpenSSL_1_1_0-stablefrom
catenacyber:overread11
Closed

Adds multiple checks to avoid buffer over reads [1.1.0]#5686
catenacyber wants to merge 1 commit intoopenssl:OpenSSL_1_1_0-stablefrom
catenacyber:overread11

Conversation

@catenacyber
Copy link
Contributor

See #5675

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Mar 20, 2018
@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Mar 20, 2018
@levitte levitte changed the title Adds multiple checks to avoid buffer over reads Adds multiple checks to avoid buffer over reads [1.1.0] Mar 20, 2018
@mattcaswell mattcaswell added this to the 1.1.0 milestone Mar 20, 2018
@richsalz richsalz added 1.1.0 approval: review pending This pull request needs review by a committer labels Mar 20, 2018
ssl/t1_trce.c Outdated
Copy link
Member

Choose a reason for hiding this comment

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

this change is not really fixing an overrun because of the condition above.
note there is a similar loop in line 1085 while (xlen > 0) {

Copy link
Member

Choose a reason for hiding this comment

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

I would'nt mind changing the other loop as well to avoid unnecessary head scratching...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, see #5675 for main discussion

Copy link
Member

@bernd-edlinger bernd-edlinger left a comment

Choose a reason for hiding this comment

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

please update this PR to match #5675

@richsalz richsalz 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 Mar 26, 2018
levitte pushed a commit that referenced this pull request Mar 27, 2018
Reviewed-by: Rich Salz <[email protected]>
Reviewed-by: Bernd Edlinger <[email protected]>
(Merged from #5686)
@bernd-edlinger
Copy link
Member

Merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants