Skip to content

Conversation

@juan-arias
Copy link
Member

Proposed changes

Add checks to make sure memory allocation is successful before continuing the process.
Suggested in AzureAD/microsoft-authentication-library-for-objc#1497

Type of change

  • Feature work
  • Bug fix
  • Documentation
  • Engineering change
  • Test
  • Logging/Telemetry

Risk

  • High – Errors could cause MAJOR regression of many scenarios. (Example: new large features or high level infrastructure changes)
  • Medium – Errors could cause regression of 1 or more scenarios. (Example: somewhat complex bug fixes, small new features)
  • Small – No issues are expected. (Example: Very small bug fixes, string changes, or configuration settings changes)

Additional information

@juan-arias juan-arias requested a review from a team as a code owner June 4, 2022 00:25
[mutData appendBytes:&size length:sizeof(size)];

uint8_t *pbDerivedKey = [self kdfCounterMode:(uint8_t*)_symmetricKey.bytes
keyDerivationKeyLength:_symmetricKey.length

Choose a reason for hiding this comment

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

This pull request does not update changelog.txt.

Please consider if this change would be noticeable to a partner or user and either update changelog.txt or resolve this conversation.

Copy link
Member

@oldalton oldalton left a comment

Choose a reason for hiding this comment

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

Added a question about NULL vs nil

fixedInput:(uint8_t*)mutData.bytes
fixedInputLength:mutData.length];

if (pbDerivedKey == nil)
Copy link
Member

Choose a reason for hiding this comment

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

nit: should this be NULL?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, updated.

ctr = 1;
keyDerivated = (uint8_t *)malloc(outputSizeBit / 8); //output is 32 bytes

if (keyDerivated == nil)
Copy link
Member

Choose a reason for hiding this comment

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

Same here, should we use NULL?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

fixedInput:fixedInput
fixedInput_length:fixedInputLength];

if (dataInput == nil)
Copy link
Member

Choose a reason for hiding this comment

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

same question here

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

{
uint8_t *tmpFixedInput = (uint8_t *)malloc(fixedInput_length + 4); //+4 is caused from the ct

if (tmpFixedInput == nil)
Copy link
Member

Choose a reason for hiding this comment

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

same here

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

@juan-arias juan-arias merged commit 92d2664 into dev Jun 8, 2022
@juan-arias juan-arias deleted the jarias/validate-allocated-memory-gh-issue branch June 8, 2022 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants