Go: memory leakage queries#39
Conversation
ex0dus-0x
force-pushed
the
alan/go_memleak
branch
from
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()
There was a problem hiding this comment.
Looks good on my items, I'd like a final sanity check from a codeowner :) @GrosQuildu maybe?
3 participants
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.