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

Backport 2.16: add missing some error checks in ECP and bignum #3513

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Jul 22, 2020

Straightforward backport of #3942

Run all the addition and subtraction tests with the result aliased to
the first operand and with the result aliased to the second operand.
Before, only some of the aliasing possibilities were tested, for only
some of the functions, with only some inputs.

Signed-off-by: Gilles Peskine <[email protected]>
Fix a memory leak in mbedtls_mpi_sub_abs when the output parameter is
aliased to the second operand (X = A - X) and the result is negative.

Signed-off-by: Gilles Peskine <[email protected]>
fix_negative allocates memory for its result. The calling site didn't
check the return value, so an out-of-memory error could lead to an
incorrect calculation. Fix this.

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added bug needs-review Every commit must be reviewed by at least two team members, needs-backports Backports are missing or are pending review and approval. component-crypto Crypto primitives and low-level interfaces labels Jul 22, 2020
@gilles-peskine-arm gilles-peskine-arm self-assigned this Jul 22, 2020
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

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

This looks all good improvements to me.

mpg
mpg previously approved these changes Sep 24, 2020
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

LGTM

@mpg mpg removed the needs-review Every commit must be reviewed by at least two team members, label Sep 24, 2020
@mpg
Copy link
Contributor

mpg commented Sep 24, 2020

The CI only fails in (1) compat.sh in Travis which was fixed in the meantime by #3590 (2) armcc license issue, which is independent from this PR.

@gilles-peskine-arm
Copy link
Contributor Author

I'm going to add a changelog entry, make the (trivial, I think) 2.7 backport, and rework #3512 to be standalone.

Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

LGTM

@gilles-peskine-arm gilles-peskine-arm changed the title 2.16 only: add missing some error checks in ECP and bignum Backport 2.16: add missing some error checks in ECP and bignum Dec 6, 2020
@gilles-peskine-arm gilles-peskine-arm removed the needs-backports Backports are missing or are pending review and approval. label Dec 6, 2020
@gilles-peskine-arm gilles-peskine-arm merged commit 8ed9ac8 into Mbed-TLS:mbedtls-2.16 Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-crypto Crypto primitives and low-level interfaces
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants