C#/Java/C++: Add change note for #3110#3421
Conversation
|
Java version: class C1 {
String f1;
C1(String f1) { this.f1 = f1; }
}
class C2 {
C1 f2;
String getF2F1() {
return this.f2.f1; // Nested field read
}
void m() {
this.f2 = new C1("taint");
sink(this.getF2F1()); // NEW: "taint" reaches here
}
} |
hvitved
changed the title
calumgrant
left a comment
calumgrant
left a comment
There was a problem hiding this comment.
Thanks for adding the change note @hvitved. Great stuff!
| class C2 | ||
| { | ||
| C1 F2; | ||
|
|
There was a problem hiding this comment.
There is an extra line here.
| through methods now takes nested field reads/writes into account. For example, the library is | ||
| able to track flow from `"taint"` to `Sink()` via the method `GetF2F1()` in | ||
| ```csharp | ||
| class C1 |
There was a problem hiding this comment.
I think the example might read better if C1 and F1 could be replaced by real-world examples like Person and Surname. Just an idea.
|
|
||
| void M() | ||
| { | ||
| this.F2 = new C1() { F1 = "taint" }; |
There was a problem hiding this comment.
this. is not really idiomatic in C# and could be removed.
| have type parameters. This means that non-generic nested types inside construced types, | ||
| such as `A<int>.B`, no longer are considered unbound generics. (Such nested types do, | ||
| however, still have relevant `.getSourceDeclaration()`s, for example `A<>.B`.) | ||
| * The data-flow library has been improved, which affects and improves most security queries. Flow |
There was a problem hiding this comment.
affects and
I would also state exactly how it improves it, for example that it will find more data-flow steps and potentially more results.
|
I'll make a separate PR for the C++ version next week, integrating it with other data-flow improvements. I'll put this issue on our team board as a way to remember it. |
If you haven't already written an example feel free to use this port of the Java one: struct C {
int f1;
};
struct C2
{
C f2;
int getf2f1() {
return f2.f1; // Nested field read
}
void m() {
f2.f1 = taint();
sink(getf2f1()); // NEW: "taint" reaches here
}
}; |
|
Thanks, @MathiasVP, for quickly porting the example. If you haven't done so already, will you also turn it into a qltest to check that it actually works? @hvitved, please use that example for C++. |
I checked that the flow was missing prior to #3110, and that we get it after merging the PR. Here's a PR with the qltest: #3426 |
hvitved
changed the title
jbj
left a comment
jbj
left a comment
There was a problem hiding this comment.
I've suggested some C/C++-specific tweaks to the change note.
Co-authored-by: Jonas Jensen <jbj@github.com>
jbj
left a comment
jbj
left a comment
There was a problem hiding this comment.
Thanks, Tom! Ping @calumgrant, who has a negative review active.
calumgrant
left a comment
calumgrant
left a comment
There was a problem hiding this comment.
Thanks for the changes @hvitved.
5 participants
@aschackmull and @jbj : We will need a similar change note for Java and C++; can you either provide me the relevant examples, or create the PRs yourself, thanks?