Go: memory leakage queries#39
Conversation
87da0a5 to
fb73b9d
Compare
There was a problem hiding this comment.
a few thoughts; also, it looks like tests are failing: https://github.com/trailofbits/codeql-queries/actions/runs/25865988084/job/76008093393?pr=39#step:6:142
| ResourceAcquisition() { | ||
| this.hasQualifiedName("os", ["Open", "OpenFile", "Create", "CreateTemp", "NewFile", "Pipe"]) | ||
| or | ||
| this.hasQualifiedName("net", ["Dial", "DialTimeout", "Listen", "ListenPacket"]) | ||
| or | ||
| this.hasQualifiedName("net", ["FileConn", "FileListener", "FilePacketConn"]) | ||
| or | ||
| this.(Method).hasQualifiedName("net", "Dialer", ["Dial", "DialContext"]) | ||
| or | ||
| this.hasQualifiedName("net/http", ["Get", "Post", "PostForm", "Head"]) | ||
| or | ||
| this.(Method).hasQualifiedName("net/http", "Client", ["Do", "Get", "Post", "PostForm", "Head"]) | ||
| or | ||
| this.hasQualifiedName("crypto/tls", ["Dial", "DialWithDialer", "Client", "Server"]) | ||
| or | ||
| this.(Method).hasQualifiedName("crypto/tls", "Dialer", "DialContext") | ||
| or | ||
| this.hasQualifiedName("compress/gzip", ["NewReader", "NewWriter", "NewWriterLevel"]) | ||
| or | ||
| this.hasQualifiedName("compress/zlib", | ||
| ["NewReader", "NewWriter", "NewWriterLevel", "NewWriterLevelDict"]) | ||
| or | ||
| this.hasQualifiedName("compress/flate", ["NewReader", "NewWriter"]) | ||
| or | ||
| this.hasQualifiedName("compress/lzw", ["NewReader", "NewWriter"]) | ||
| or | ||
| this.hasQualifiedName("archive/zip", "OpenReader") | ||
| } |
There was a problem hiding this comment.
Can this be parameterized with MaD?
There was a problem hiding this comment.
Switched to using parameterized MaD for deferred release.
| // defer resp.Body.Close() — base is a selector, take its base identifier | ||
| result.asExpr() = base.(SelectorExpr).getBase().(Ident) |
There was a problem hiding this comment.
thinking out loud: can selectors be nested? does this work for e.g. a.b.c.Close()?
i think this doesn't matter for resources in the current list. net/http is a bit of an odd duck since Close isn't on the top-level Response
There was a problem hiding this comment.
Switched to using the recursive util call for selector expr. This will address nested Close()
Add two Go queries for detecting behaviors that can lead to memory leakage:
DeferReleaseInLoop- deferring a resource release can cause memory leakage across iterations. This query models constrains search to common APIs where this would manifest (eg.os.OpenFile).UnboundedIORead- invokingio.ReadAllof untrusted input can exhaust server memory for a denial-of-service.