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

Fix out-of-bounds vector accesses in Pruner::enforce. #398

Merged
merged 1 commit into from Dec 14, 2019
Merged

Fix out-of-bounds vector accesses in Pruner::enforce. #398

merged 1 commit into from Dec 14, 2019

Conversation

jamesjer
Copy link
Contributor

I have been testing builds for fplll 5.3.0 and fpylll 0.5.0dev for Fedora. The default Fedora build flags include -D_GLIBCXX_ASSERTIONS. This led to test failures in fpylll due to out of bounds vector accesses in the fplll code. This commit fixes the two such out of bounds accesses found by the fpylll tests. (I highly recommend that you build with -D_GLIBCXX_ASSERTIONS before running your tests, by the way.)

In the first hunk, note that min_pruning_coefficients always has length d, not n. The fpylll test suite, at least, invokes this code in such a way that i / c can be equal to d and therefore one past the end of the min_pruning_coefficients vector.

In the second hunk, j can be as large as as dn (b.size()), and therefore on the first iteration of the loop, when i is j - 1, then b[i + 1] == b[j] is one past the end of b.

@codecov
Copy link

codecov bot commented Dec 2, 2019

Codecov Report

Merging #398 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #398   +/-   ##
=======================================
  Coverage   69.25%   69.25%           
=======================================
  Files          67       67           
  Lines        4993     4993           
=======================================
  Hits         3458     3458           
  Misses       1535     1535
Impacted Files Coverage Δ
fplll/pruner/pruner.h 58% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7439f1d...f68e257. Read the comment docs.

@shi-bai
Copy link
Collaborator

shi-bai commented Dec 10, 2019

James, thanks for the report.

On my debian, I cannot trigger the error by enabling D_GLIBCXX_ASSERTIONS in configuration. The tests seem to pass.

Testsuite summary for fplll 5.3.0

TOTAL: 13

PASS: 13

SKIP: 0

XFAIL: 0

FAIL: 0

XPASS: 0

ERROR: 0

Does the checking error comes from Line 22 of test_pruner.cpp?

Shi

@malb
Copy link
Collaborator

malb commented Dec 13, 2019

Over at fplll/fpylll#161 it seems to remove some non deterministic behaviour, so I'm inclined to merge this soon. Shi, do you sign off on this?

@shi-bai
Copy link
Collaborator

shi-bai commented Dec 14, 2019

I could not repeat this error on my debian. But see fplll/fpylll#161 that removes error. Happy to merge.

@shi-bai shi-bai merged commit 7d696e9 into fplll:master Dec 14, 2019
@malb
Copy link
Collaborator

malb commented Dec 14, 2019

Ta!

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.

None yet

3 participants