Skip to content

add minimum matrix size to reduce disclosure risk - #500

Merged
timcadman merged 2 commits into
refactor/perf-batch-5from
fix/matrix-disclosure
Jul 27, 2026
Merged

add minimum matrix size to reduce disclosure risk#500
timcadman merged 2 commits into
refactor/perf-batch-5from
fix/matrix-disclosure

Conversation

@timcadman

@timcadmantimcadman commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Background

matrixDetDS1 is the only matrix aggregate function, but it has no disclosure checks. It could be potentially disclosive for small matrices.

What's changed

Throw an error if the number of rows of the input matrix is < nfilter.subset

How to test

  • Check disclosure control level is correct
  • Check CI green

@timcadman
timcadman marked this pull request as draft July 23, 2026 15:05
@timcadman

Copy link
Copy Markdown
ContributorAuthor

Sorry stuart this is a mess, just fixing it!

@timcadman
timcadman requested review from StuartWheater and removed request for StuartWheaterJuly 23, 2026 15:06
@timcadman
timcadman changed the base branch from v7.0-dev to refactor/perf-batch-5July 23, 2026 15:25
@timcadman
timcadmanforce-pushed the fix/matrix-disclosure branch from cec2dc2 to 7cb57fcCompareJuly 23, 2026 15:27
@timcadman
timcadman marked this pull request as ready for review July 23, 2026 15:28
@timcadman

Copy link
Copy Markdown
ContributorAuthor

Ok fixed!

@StuartWheater

Copy link
Copy Markdown
Member

Ack. Will review after tests complete.

@StuartWheaterStuartWheater left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • I wonder if it does need a separate nfilter value, not reusing subset
  • Gain we have the problem if the input matrix has been maliciously constructed.

@timcadman

Copy link
Copy Markdown
ContributorAuthor

Agreed - nfiltersubset wasn't ideal, but it seemed a smaller change than adding a new filter. You're right - it doesn't protect from the malicious attack, hence why I included it in this disclosure profile.

@timcadman
timcadman merged commit 670a9f8 into refactor/perf-batch-5Jul 27, 2026
9 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@timcadman@StuartWheater