Skip to content

fix(kernels): add array overflow composer error#762

Merged
Maddiaa0 merged 8 commits into
masterfrom
md/array-push-assertions
Jun 7, 2023
Merged

fix(kernels): add array overflow composer error#762
Maddiaa0 merged 8 commits into
masterfrom
md/array-push-assertions

Conversation

@Maddiaa0

@Maddiaa0 Maddiaa0 commented Jun 6, 2023

Copy link
Copy Markdown
Member

Description

Swaps out push_array_to_array ASSERT statements with composer errors, right now appending arrays of equal sizes will silently fail and overflow, overwriting memory in places that it shouldn't. e.g. adding two commitment arrays of max size will cause them to overflow into the nullifiers.

This change creates a composer kernel error for this case.

ASSERT(source_size <= size_2 - target_size);

Please provide a paragraph or two giving a summary of the change, including relevant motivation and context.

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • The branch has been merged or rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.

@Maddiaa0 Maddiaa0 force-pushed the md/array-push-assertions branch from 3d6a86b to 29ba730 Compare June 6, 2023 15:56
@Maddiaa0 Maddiaa0 marked this pull request as ready for review June 6, 2023 17:54
@Maddiaa0 Maddiaa0 requested a review from LHerskind June 6, 2023 17:54
@Maddiaa0 Maddiaa0 force-pushed the md/array-push-assertions branch from a24dda1 to 5364b22 Compare June 6, 2023 18:42
@Maddiaa0 Maddiaa0 changed the title fix: add array overflow composer error fix(circuits): add array overflow composer error Jun 6, 2023
@Maddiaa0 Maddiaa0 changed the title fix(circuits): add array overflow composer error fix(kernels): add array overflow composer error Jun 6, 2023
#include "aztec3/circuits/abis/public_kernel/public_kernel_inputs.hpp"
#include "aztec3/circuits/abis/public_kernel/public_kernel_inputs_no_previous_kernel.hpp"
#include "aztec3/circuits/hash.hpp"
#include "aztec3/constants.hpp"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you need the constants for here?

@Maddiaa0 Maddiaa0 enabled auto-merge (squash) June 7, 2023 10:50
@Maddiaa0 Maddiaa0 merged commit 7c936b3 into master Jun 7, 2023
@Maddiaa0 Maddiaa0 deleted the md/array-push-assertions branch June 7, 2023 11: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.

2 participants