Rust: Update legacy MaD models 1 #19934
Conversation
| let path = e.path(); // $ MISSING: Alert[rust/summary/taint-sources] | ||
| let file_name = e.file_name(); // $ MISSING: Alert[rust/summary/taint-sources] | ||
| sink(path); // $ MISSING: hasTaintFlow | ||
| sink(file_name); // $ MISSING: hasTaintFlow |
There was a problem hiding this comment.
@hvitved I'm looking to get these results back. The sources go missing because there's no result for getCanonicalPath on lines 423 and 424, I'm guessing that means we haven't inferred the type of e / entry due to limitations of type inference on for loops (line 421).
If you agree that's the problem, do you have an idea what the general case for inferForLoopExprType should look like? I think we need to get the type of the Iterator (returned by fs::read_dir in this case) and pull it apart for it's Item type.
There was a problem hiding this comment.
You are right, it must be because we cannot currently infer the type of e, which means we cannot resolve the e.path() call (note that it is not e.path() itself that has a canonical path, it is the (missing) target that has a canonical path).
If you agree that's the problem, do you have an idea what the general case for inferForLoopExprType should look like? I think we need to get the type of the Iterator (returned by fs::read_dir in this case) and pull it apart for it's Item type.
Correct; I will give it a try.
There was a problem hiding this comment.
Thanks. Let me know if there's anything I can do to help.
|
DCA results - interestingly enough DCA does not show a loss of sources, it actually recognises a handful more, though a significant number of query sinks and 4 query results are lost. The lost results are the FPs we gained in #19881, I'm not too worried about them right now. The lost sinks I've narrowed down to the I don't think we should insist on fixing these now. I do think we need continue putting effort into getting type inference, call targets and canonical paths working in more cases to recover these losses. |
| let path = e.path(); // $ MISSING: Alert[rust/summary/taint-sources] | ||
| let file_name = e.file_name(); // $ MISSING: Alert[rust/summary/taint-sources] | ||
| sink(path); // $ MISSING: hasTaintFlow | ||
| sink(file_name); // $ MISSING: hasTaintFlow |
There was a problem hiding this comment.
You are right, it must be because we cannot currently infer the type of e, which means we cannot resolve the e.path() call (note that it is not e.path() itself that has a canonical path, it is the (missing) target that has a canonical path).
If you agree that's the problem, do you have an idea what the general case for inferForLoopExprType should look like? I think we need to get the type of the Iterator (returned by fs::read_dir in this case) and pull it apart for it's Item type.
Correct; I will give it a try.
Update some legacy MaD models to the new model format. This PR does perhaps a third of the models, I have more unfinished translations locally but I want to start pushing some of this work through CI + DCA and find out if there are going to be any issues there.