Skip to content

Commit 33b029e

Browse files
committed
fix(security): confine GetCertHash to known cert files (CWE-22)
Resolve CodeQL go/path-injection (alert #96): the certFile path from the getCertHash endpoint flowed straight into os.ReadFile, letting an authenticated request read arbitrary files by path. Validate it against an allow-list of certificate files the panel already references (inbound TLS certificateFile values plus the panel's own web cert) and read the config-sourced path rather than the caller-supplied one, breaking the taint flow while preserving arbitrary cert locations.
1 parent dfd77ca commit 33b029e

1 file changed

Lines changed: 79 additions & 1 deletion

File tree

internal/web/service/server.go

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1735,7 +1735,16 @@ func (s *ServerService) GetNewmldsa65() (any, error) {
17351735
func (s *ServerService) GetCertHash(certFile string, certContent string) ([]string, error) {
17361736
var certBytes []byte
17371737
if path := strings.TrimSpace(certFile); path != "" {
1738-
b, err := os.ReadFile(path)
1738+
// Guard against path traversal: only hash certificate files the panel
1739+
// already references in its own configuration (an inbound's TLS
1740+
// certificateFile or the panel's own web cert). The path handed to
1741+
// os.ReadFile comes from that allow-list, never directly from the
1742+
// caller-supplied value.
1743+
known, ok := s.resolveKnownCertFile(path)
1744+
if !ok {
1745+
return nil, common.NewError("certificate file is not referenced by any inbound or panel setting")
1746+
}
1747+
b, err := os.ReadFile(known)
17391748
if err != nil {
17401749
return nil, err
17411750
}
@@ -1781,6 +1790,75 @@ func (s *ServerService) GetCertHash(certFile string, certContent string) ([]stri
17811790
return hashes, nil
17821791
}
17831792

1793+
// resolveKnownCertFile checks the caller-supplied certificate path against the
1794+
// set of certificate files the panel already references (inbound TLS configs
1795+
// plus the panel's own web cert) and, on a match, returns the path taken from
1796+
// that configuration — not the caller's value. This both confines reads to
1797+
// known certificates and breaks the user-input-to-filesystem taint flow.
1798+
func (s *ServerService) resolveKnownCertFile(certFile string) (string, bool) {
1799+
want := filepath.Clean(certFile)
1800+
for _, known := range s.knownCertFiles() {
1801+
if filepath.Clean(known) == want {
1802+
return known, true
1803+
}
1804+
}
1805+
return "", false
1806+
}
1807+
1808+
// knownCertFiles collects every certificate file path the panel legitimately
1809+
// references: the certificateFile of each inbound's TLS settings and the
1810+
// panel's own web TLS certificate.
1811+
func (s *ServerService) knownCertFiles() []string {
1812+
var files []string
1813+
if cert, err := s.settingService.GetCertFile(); err == nil {
1814+
if cert = strings.TrimSpace(cert); cert != "" {
1815+
files = append(files, cert)
1816+
}
1817+
}
1818+
if inbounds, err := s.inboundService.GetAllInbounds(); err == nil {
1819+
for _, inbound := range inbounds {
1820+
files = collectCertFiles(inbound.StreamSettings, files)
1821+
}
1822+
}
1823+
return files
1824+
}
1825+
1826+
// collectCertFiles walks a stream-settings JSON document and appends the value
1827+
// of every "certificateFile" field it finds (TLS settings may nest them under
1828+
// several keys depending on the security type).
1829+
func collectCertFiles(streamSettings string, out []string) []string {
1830+
streamSettings = strings.TrimSpace(streamSettings)
1831+
if streamSettings == "" {
1832+
return out
1833+
}
1834+
var parsed any
1835+
if err := json.Unmarshal([]byte(streamSettings), &parsed); err != nil {
1836+
return out
1837+
}
1838+
return walkCertFiles(parsed, out)
1839+
}
1840+
1841+
func walkCertFiles(node any, out []string) []string {
1842+
switch v := node.(type) {
1843+
case map[string]any:
1844+
for key, val := range v {
1845+
if key == "certificateFile" {
1846+
if path, ok := val.(string); ok {
1847+
if path = strings.TrimSpace(path); path != "" {
1848+
out = append(out, path)
1849+
}
1850+
}
1851+
}
1852+
out = walkCertFiles(val, out)
1853+
}
1854+
case []any:
1855+
for _, item := range v {
1856+
out = walkCertFiles(item, out)
1857+
}
1858+
}
1859+
return out
1860+
}
1861+
17841862
// GetRemoteCertHash runs `xray tls ping <server>` to fetch the live certificate
17851863
// SHA-256 of a remote endpoint — the value to put in pinnedPeerCertSha256 (pcs)
17861864
// when pinning a server whose certificate file you don't hold (a CDN front, a

0 commit comments

Comments
 (0)