github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.75k stars 1.56k forks source link

False positive: CWE-918 #9306

Open xssssrf opened 3 years ago

xssssrf commented 3 years ago

Test case:

package main

import (
    "net/http"
    "net/url"
)

func testssrf(req *http.Request) {
    host := req.URL.Query().Get("host")

    u, _ := url.Parse("http://example")

    // The current value of u is "http://example"
    http.Get(u.String())  // Expected: negative  Actual: positive

    u.Host = host
    http.Get(u.String())
}

The following code may have caused the false positives. codeql-go/ql/src/semmle/go/security/RequestForgery.qll

  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
    // propagate to a URL when its host is assigned to
    exists(Write w, Field f, SsaWithFields v | f.hasQualifiedName("net/url", "URL", "Host") |
      w.writesField(v.getAUse(), f, pred) and succ = v.getAUse()
    )
  }
smowton commented 3 years ago

Acknowledged, this is a known limitation of how we currently model side-effects in Go. Is this causing you problems in practice, or is it just an oddity you noticed in synthetic code?

xssssrf commented 3 years ago

Acknowledged, this is a known limitation of how we currently model side-effects in Go. Is this causing you problems in practice, or is it just an oddity you noticed in synthetic code?

Just in synthetic code.