Skip to content

Conversation

sauyon
Copy link
Contributor

@sauyon sauyon commented Sep 24, 2018

This probably means this is a hack to do something like set a bunch of struct fields at once. See: https://lgtm.com/projects/g/CZ-NIC/bird/snapshot/9ba0b39e4c4756944718ab9cb55efd5824527a4b/files/proto/rip/packets.c#x17ea4190ce37e101:1.

@geoffw0

geoffw0
geoffw0 previously approved these changes Sep 25, 2018
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

I'm happy with this solution. It reminds me of the memberMayBeVarSize mechanism from Buffer.qll, which will catch arrays of size 0 or 1 at the end of a struct - but not arrays of size 0 elsewhere in the struct as in the case you've linked to. It's actually quite a strange case and I won't pretend to understand why they chose to do it that way.

@@ -21,4 +21,5 @@ from BufferWrite bw, int destSize
where bw.hasExplicitLimit() // has an explicit size limit
and destSize = getBufferSize(bw.getDest(), _)
and (bw.getExplicitLimit() > destSize) // but it's larger than the destination
and not destSize = 0 // probably just a hack if the destination size is 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rephrase 'hack' to something more neutral / informative? Perhaps 'likely to be a variable-sized member or similar pattern'.

@geoffw0 geoffw0 added the C++ label Sep 25, 2018
@geoffw0
Copy link
Contributor

geoffw0 commented Oct 8, 2018

I'm going to fix the issues and make a fresh PR of this, assuming nobody's already done so.

@sauyon
Copy link
Contributor Author

sauyon commented Oct 8, 2018

Sorry; I've been a bit busy moving back into Oxford. I'll do that now.

@geoffw0
Copy link
Contributor

geoffw0 commented Oct 8, 2018

I've now opened #293 which includes and extends this work.

@geoffw0 geoffw0 closed this Oct 8, 2018
@geoffw0
Copy link
Contributor

geoffw0 commented Oct 8, 2018

Thanks!

@sauyon
Copy link
Contributor Author

sauyon commented Oct 8, 2018

ah, ok, cool!

aibaars pushed a commit that referenced this pull request Oct 14, 2021
rb/sql-injection: fix FPs stemming from not accounting for overridden methods
smowton added a commit to smowton/codeql that referenced this pull request Feb 7, 2022
…odos

Log when a class version can't be read
MathiasVP pushed a commit to MathiasVP/ql that referenced this pull request Aug 10, 2025
Co-authored-by: Denis Levin <denisl@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants