Skip to content

fix: close leaked file handles in config and token key loading#1680

Open
fereidani wants to merge 2 commits into
cloudflare:masterfrom
fereidani:fix/resource-leaks
Open

fix: close leaked file handles in config and token key loading#1680
fereidani wants to merge 2 commits into
cloudflare:masterfrom
fereidani:fix/resource-leaks

Conversation

@fereidani

Copy link
Copy Markdown

Hi,

I've built a static analyzer for Go called Ghoul, and I ran it against the cloudflared codebase. It flagged a couple of file handles that are opened but never closed, which can leak file descriptors over the lifetime of the process (and across config reloads). This PR closes both.

Ghoul related reports:

config/configuration.go:422:2: [resource-leak] resource from os.Open allocated at config/configuration.go:413 never closed (confidence: 6.4/10, CWE-404)
  --> config/configuration.go:413:25: source:      resource allocated by os.Open
token/encrypt.go:159:3: [resource-leak] resource from os.Open allocated at token/encrypt.go:150 never closed (confidence: 6.4/10, CWE-404)
  --> token/encrypt.go:150:19: source:      resource allocated by os.Open
token/encrypt.go:164:2: [resource-leak] resource from os.Open allocated at token/encrypt.go:150 never closed (confidence: 6.4/10, CWE-404)
  --> token/encrypt.go:150:19: source:      resource allocated by os.Open

Changes:

  • config/configuration.go: the strict-mode re-parse in ReadConfigFile opens the config file a second time without closing it. Added a deferred close so the handle is released on every path.
  • token/encrypt.go: Encrypter.fetchKey opens the key file but never closes it on any return path. Added a deferred close.

Both use the project's defer func() { _ = file.Close() }() convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant