Go: DotDotReplaceAll ignores the replacement value and causes false-negative path-injection alerts

**Description of the issue**

The Go `go/path-injection` query treats expressions modeled by
`StringOps::ReplaceAll` as sanitized when the replaced string is `"."` or
`".."`. The sanitizer does not check the replacement string or establish that
the resulting path is relative or contained within a trusted directory.

This causes false negatives for at least two independent reasons:

1. The replacement can make the path more dangerous, because the model checks
only the string being replaced and not the replacement value.
2. Even replacing `".."` with the empty string can leave or create an
attacker-controlled absolute path.

**Affected sanitizer**

[`DotDotReplaceAll` in `TaintedPathCustomizations.qll`](https://github.com/github/codeql/blob/42843f155e95d25e690aba9ae4620b5a5986a951/go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll#L124-L130):

```ql
/**
* A replacement of the form `!strings.ReplaceAll(nd, "..")` or
* `!strings.ReplaceAll(nd, ".")`, considered as a sanitizer for path traversal.
*/
class DotDotReplaceAll extends StringOps::ReplaceAll, Sanitizer {
DotDotReplaceAll() { this.getReplacedString() = ["..", "."] }
}
```

`StringOps::ReplaceAll.getReplacedString()` represents the `old` argument. For
`strings.ReplaceAll(s, old, new)`, the model exposes and checks `old`, but not
`new`:

[`StringOps.qll`](https://github.com/github/codeql/blob/42843f155e95d25e690aba9ae4620b5a5986a951/go/ql/lib/semmle/go/StringOps.qll#L170-L180):

```ql
class ReplaceAll extends DataFlow::Node instanceof ReplaceAll::Range {
/** Gets the `old` in `strings.ReplaceAll(s, old, new)`. */
string getReplacedString() { result = super.getReplacedString() }
}
```

The [standard-library implementation of this range](https://github.com/github/codeql/blob/42843f155e95d25e690aba9ae4620b5a5986a951/go/ql/lib/semmle/go/StringOps.qll#L198-L213)
similarly obtains only argument 1 (`old`):

```ql
override string getReplacedString() { result = this.getArgument(1).getStringValue() }
```

Consequently, any replacement value is accepted by `DotDotReplaceAll`.

**False negative when the replacement increases traversal**

For example:

```go
func handler(w http.ResponseWriter, r *http.Request) {
taintedPath := r.URL.Query().Get("path")
path := strings.ReplaceAll(taintedPath, "..", "../..")
data, _ := os.ReadFile(path) // expected: go/path-injection
w.Write(data)
}
```

For `taintedPath = "../secret"`, the replacement produces
`"../../secret"`. It increases the number of parent-directory components, but
CodeQL treats the return value as sanitized because the `old` argument is
`".."`.

I also verified this directly in the existing `TaintedPath.go` test by changing
the replacement at line 37 to `"../.."` and adding an expected
`go/path-injection` alert. With `DotDotReplaceAll` enabled, the test reports:

```text
| TaintedPath.go:37:77:37:105 | comment | Missing result: Alert[go/path-injection] |
```

**False negative with an empty replacement**

The existing test uses an empty replacement:

[`TaintedPath.go` lines 36-38](https://github.com/github/codeql/blob/42843f155e95d25e690aba9ae4620b5a5986a951/go/ql/test/query-tests/Security/CWE-022/TaintedPath.go#L36-L38):

```go
// GOOD: Sanitized by strings.ReplaceAll and replaces all .. with empty string
data, _ = ioutil.ReadFile(strings.ReplaceAll(tainted_path, "..", ""))
w.Write(data)
```

Removing `".."` does not ensure that the path is safe:

```go
strings.ReplaceAll("..../etc/passwd", "..", "") // "/etc/passwd"
strings.ReplaceAll("/etc/passwd", "..", "") // "/etc/passwd"
```

In both cases the result is an attacker-controlled absolute path. This is
consistent with the query help, which says that absolute paths can point
anywhere on the file system and similarly warns that naive removal of traversal
sequences can be insufficient:

- [`TaintedPath.qhelp`: absolute paths can point anywhere on the file system](https://github.com/github/codeql/blob/42843f155e95d25e690aba9ae4620b5a5986a951/go/ql/src/Security/CWE-022/TaintedPath.qhelp#L8-L16)
- [`TaintedPath.qhelp`: warning about naive traversal-sequence removal](https://github.com/github/codeql/blob/42843f155e95d25e690aba9ae4620b5a5986a951/go/ql/src/Security/CWE-022/TaintedPath.qhelp#L24-L39)

**Steps to reproduce using the existing test**

1. Run the existing test:

```text
codeql test run go/ql/test/query-tests/Security/CWE-022/TaintedPath.qlref
```

The test passes and no alert is reported for the `ReplaceAll` result at
`TaintedPath.go` line 37.

2. Remove only the `DotDotReplaceAll` class from
`go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll` and rerun the
same test.

3. The test fails with the new source-to-sink result:

```text
| TaintedPath.go:37:28:37:69 | call to ReplaceAll | ... | user-provided value |
| TaintedPath.go:37:28:37:69 | call to ReplaceAll | Unexpected result: Alert |
```

The individual ablation changes no other alert locations in this test. This
confirms that `DotDotReplaceAll` suppresses the flow from the URL-derived path
through `strings.ReplaceAll` to `ioutil.ReadFile`.

**Expected behavior**

Replacing `"."` or `".."` should not be treated as an unconditional
`path-injection` sanitizer. In particular, a replacement operation must not
stop taint without considering the replacement value and the properties of the
resulting path.

Since even an empty replacement does not prove that the result is relative or
contained within a safe directory, a possible fix is to remove
`DotDotReplaceAll` and update the affected test expectation. Sound validation
can instead rely on checks that establish locality or containment.

**Environment**

- CodeQL CLI 2.25.6
- CodeQL repository test baseline: `f6f45d1536`
- Present in `main` at commit `42843f155e95d25e690aba9ae4620b5a5986a951`
- Go 1.22.12 on Linux/amd64

Start with DotDotReplaceAll in go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll and the ReplaceAll model in StringOps.qll, then run codeql test run go/ql/test/query-tests/Security/CWE-022/TaintedPath.qlref. Check the affected cases in TaintedPath.go and update the model or expectations so unsafe ReplaceAll results produce the expected go/path-injection alert without suppressing valid sanitizers.

Escrito por el modelo de indexación a partir del texto del issue.