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

Type fix in mldsa #2308

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Type fix in mldsa #2308

wants to merge 4 commits into from

Conversation

manastasova
Copy link
Contributor

Change types uint32_t t0, t1; to int32_t t0, t1; due to potential overflow in if (t0 < 9){a[ctr++] = 4 - t0;} causing cbmc proofs to fail.

Issues:

From pq-code-package/mldsa-native#86.

Description of changes:

The output array is of type int32_t* a, thus, uint32_t aux values t0, t1 cause cbmc proofs to fail due to potential overflow.

Testing:

./crypto/crypto_test

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

The resulting array is of type 'int32_t* a', thus, uint32_t aux values t0, t1 cause cbmc proofs to fail due to potentional overflow.
@manastasova manastasova requested a review from a team as a code owner April 2, 2025 00:21
@hanno-becker
Copy link
Contributor

hanno-becker commented Apr 2, 2025

@manastasova Why not include an explicit cast here prior to 2 - t_i and 4 - t_i? The computation of t0,t1 is unsigned, I think, and you would want CBMC to check that it does not underflow.

This PR should wait until the corresponding PR pq-code-package/mldsa-native#86 is merged.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.75%. Comparing base (54453b4) to head (38991b5).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2308      +/-   ##
==========================================
- Coverage   78.75%   78.75%   -0.01%     
==========================================
  Files         616      616              
  Lines      107650   107649       -1     
  Branches    15303    15300       -3     
==========================================
- Hits        84777    84776       -1     
+ Misses      22219    22218       -1     
- Partials      654      655       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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