Skip to content

Commit f707966

Browse files
ericfitzclaude
ericfitz
and
claude
authored
feat(security): add CodeQL query pack for slogging sanitizer awareness (#105)
Add custom CodeQL query pack that teaches CodeQL about the TMI slogging package's built-in log injection sanitization (CWE-117). The pack models: - SanitizeLogMessage() as a sanitizer that breaks taint flow - Logger, ContextLogger, and FallbackLogger methods as safe sinks This prevents false positives for log injection alerts since all slogging methods internally call SanitizeLogMessage() before writing to logs. Files added: - .github/codeql/custom-queries/qlpack.yml - .github/codeql/custom-queries/go/slogging/SloggingSanitizers.qll - .github/codeql/custom-queries/go/slogging/LogInjectionWithSlogging.ql Updated codeql-config.yml to: - Include the custom query pack - Exclude the default go/log-injection query (replaced by our custom version) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent b29bc09 commit f707966

File tree

6 files changed

+212
-19
lines changed

6 files changed

+212
-19
lines changed

‎.github/codeql/codeql-config.yml‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ name: "TMI CodeQL Config"
77
queries:
88
- uses: security-extended
99
- uses: security-and-quality
10+
# Custom queries that understand TMI's slogging sanitizer
11+
- uses: ./.github/codeql/custom-queries
1012

1113
# Path filters - exclude generated code and development-only scripts
1214
paths-ignore:
@@ -24,28 +26,32 @@ query-filters:
2426
# These are development-only tools that intentionally log OAuth tokens for debugging
2527
- exclude:
2628
id: py/clear-text-logging-sensitive-data
29+
# Exclude default go/log-injection query - replaced by our custom query
30+
# that understands the slogging package's SanitizeLogMessage() sanitizer
31+
- exclude:
32+
id: go/log-injection
2733

2834
# =============================================================================
29-
# RESOLVED - Log Injection (go/log-injection) - FIXED IN CODE
35+
# RESOLVED - Log Injection (go/log-injection) - CUSTOM QUERY PACK
3036
# =============================================================================
31-
# As of this commit, the TMI slogging package now sanitizes all log messages
32-
# by calling SanitizeLogMessage() which removes newlines, carriage returns,
33-
# and tabs that could be used for log injection attacks (CWE-117).
37+
# The TMI slogging package sanitizes all log messages by calling
38+
# SanitizeLogMessage() which removes newlines, carriage returns, and tabs
39+
# that could be used for log injection attacks (CWE-117).
3440
#
35-
# Files updated:
36-
# - internal/slogging/logger.go: Logger.Debug/Info/Warn/Error methods
37-
# - internal/slogging/context.go: FallbackLogger and ContextLogger methods
41+
# CodeQL's default go/log-injection query doesn't understand that slogging
42+
# methods are safe sinks. We've created a custom CodeQL query pack that:
3843
#
39-
# The sanitization happens at the logging layer, so all callers automatically
40-
# benefit from this protection without needing to modify their code.
44+
# 1. Models SanitizeLogMessage() as a sanitizer that breaks taint flow
45+
# 2. Models all slogging methods (Logger, ContextLogger, FallbackLogger)
46+
# as safe sinks since they call SanitizeLogMessage() internally
4147
#
42-
# After the next CodeQL scan, existing alerts should be resolved because:
43-
# 1. User input flows into logger.Debug/Info/Warn/Error calls
44-
# 2. These methods now call SanitizeLogMessage() before logging
45-
# 3. SanitizeLogMessage() strips control characters, breaking the taint flow
48+
# Custom query files:
49+
# - .github/codeql/custom-queries/go/slogging/SloggingSanitizers.qll
50+
# - .github/codeql/custom-queries/go/slogging/LogInjectionWithSlogging.ql
4651
#
47-
# If alerts persist after the fix is merged, they can be dismissed as
48-
# "Fixed in code" with reference to this configuration comment.
52+
# The default go/log-injection query is excluded above and replaced by our
53+
# custom query (go/log-injection-slogging-aware) which understands the
54+
# slogging sanitization.
4955
# =============================================================================
5056

5157
# =============================================================================

‎.github/codeql/custom-queries/go/slogging/LogInjectionWithSlogging.ql‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Log injection (with slogging sanitizer awareness)
3+
* @description Building log entries from user-controlled data may allow
4+
* attackers to forge log entries, obscure security events,
5+
* or corrupt log files. This query is aware of the TMI slogging
6+
* package's built-in sanitization.
7+
* @kind path-problem
8+
* @problem.severity warning
9+
* @security-severity 7.8
10+
* @precision high
11+
* @id go/log-injection-slogging-aware
12+
* @tags security
13+
* external/cwe/cwe-117
14+
*/
15+
16+
import go
17+
import semmle.go.security.LogInjection
18+
import SloggingSanitizers
19+
import LogInjection::Flow::PathGraph
20+
21+
from LogInjection::Flow::PathNode source, LogInjection::Flow::PathNode sink
22+
where LogInjection::Flow::flowPath(source, sink)
23+
select sink.getNode(), source, sink, "Log entry depends on a $@.", source.getNode(),
24+
"user-provided value"

‎.github/codeql/custom-queries/go/slogging/SloggingSanitizers.qll‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
/**
2+
* Models the TMI slogging package as providing sanitization for log injection attacks.
3+
*
4+
* The slogging package (github.com/ericfitz/tmi/internal/slogging) provides structured
5+
* logging with built-in sanitization via SanitizeLogMessage(). This function removes
6+
* newlines, carriage returns, and tabs that could be used for log injection attacks (CWE-117).
7+
*
8+
* All logging methods in the package call SanitizeLogMessage() before writing to the log:
9+
* - Logger.Debug/Info/Warn/Error
10+
* - Logger.DebugCtx/InfoCtx/WarnCtx/ErrorCtx
11+
* - ContextLogger.Debug/Info/Warn/Error
12+
* - ContextLogger.DebugCtx/InfoCtx/WarnCtx/ErrorCtx
13+
* - FallbackLogger.Debug/Info/Warn/Error
14+
*
15+
* This library teaches CodeQL that:
16+
* 1. SanitizeLogMessage() is a sanitizer that breaks taint flow for log injection
17+
* 2. The logging methods in slogging are safe sinks because they sanitize internally
18+
*/
19+
20+
import go
21+
import semmle.go.dataflow.DataFlow
22+
import semmle.go.security.LogInjection
23+
24+
/**
25+
* The slogging package path.
26+
*/
27+
private string sloggingPackage() { result = "github.com/ericfitz/tmi/internal/slogging" }
28+
29+
/**
30+
* A call to SanitizeLogMessage in the slogging package.
31+
* This function sanitizes log messages by removing control characters.
32+
*/
33+
class SanitizeLogMessageCall extends DataFlow::CallNode {
34+
SanitizeLogMessageCall() {
35+
this.getTarget().hasQualifiedName(sloggingPackage(), "SanitizeLogMessage")
36+
}
37+
38+
/** Gets the input argument being sanitized. */
39+
DataFlow::Node getInput() { result = this.getArgument(0) }
40+
41+
/** Gets the sanitized output. */
42+
DataFlow::Node getOutput() { result = this.getResult() }
43+
}
44+
45+
/**
46+
* Models SanitizeLogMessage as a sanitizer for log injection.
47+
* Data flowing through this function is considered sanitized.
48+
*/
49+
class SloggingSanitizer extends LogInjection::Sanitizer {
50+
SloggingSanitizer() {
51+
exists(SanitizeLogMessageCall call | this = call.getOutput())
52+
}
53+
}
54+
55+
/**
56+
* Models calls to slogging Logger methods as safe sinks.
57+
* These methods internally call SanitizeLogMessage before logging.
58+
*/
59+
class SloggingLoggerMethod extends DataFlow::CallNode {
60+
string methodName;
61+
62+
SloggingLoggerMethod() {
63+
exists(Method m |
64+
m.hasQualifiedName(sloggingPackage(), "Logger", methodName) and
65+
methodName in ["Debug", "Info", "Warn", "Error", "DebugCtx", "InfoCtx", "WarnCtx", "ErrorCtx"] and
66+
this = m.getACall()
67+
)
68+
}
69+
70+
/** Gets the format/message argument. */
71+
DataFlow::Node getMessageArg() {
72+
// First argument is always the format string or message
73+
result = this.getArgument(0)
74+
or
75+
// For *Ctx methods, first arg is context, second is message
76+
methodName.matches("%Ctx") and result = this.getArgument(1)
77+
}
78+
}
79+
80+
/**
81+
* Models calls to slogging ContextLogger methods as safe sinks.
82+
* These methods internally call SanitizeLogMessage before logging.
83+
*/
84+
class SloggingContextLoggerMethod extends DataFlow::CallNode {
85+
string methodName;
86+
87+
SloggingContextLoggerMethod() {
88+
exists(Method m |
89+
m.hasQualifiedName(sloggingPackage(), "ContextLogger", methodName) and
90+
methodName in ["Debug", "Info", "Warn", "Error", "DebugCtx", "InfoCtx", "WarnCtx", "ErrorCtx"] and
91+
this = m.getACall()
92+
)
93+
}
94+
95+
/** Gets the format/message argument. */
96+
DataFlow::Node getMessageArg() {
97+
result = this.getArgument(0)
98+
}
99+
}
100+
101+
/**
102+
* Models calls to slogging FallbackLogger methods as safe sinks.
103+
* These methods internally call SanitizeLogMessage before logging.
104+
*/
105+
class SloggingFallbackLoggerMethod extends DataFlow::CallNode {
106+
string methodName;
107+
108+
SloggingFallbackLoggerMethod() {
109+
exists(Method m |
110+
m.hasQualifiedName(sloggingPackage(), "FallbackLogger", methodName) and
111+
methodName in ["Debug", "Info", "Warn", "Error"] and
112+
this = m.getACall()
113+
)
114+
}
115+
116+
/** Gets the format/message argument. */
117+
DataFlow::Node getMessageArg() {
118+
result = this.getArgument(0)
119+
}
120+
}
121+
122+
/**
123+
* Extends the LogInjection sanitizer to recognize data flowing into slogging methods.
124+
* Since all slogging methods call SanitizeLogMessage internally, any data that reaches
125+
* a slogging method is effectively sanitized before being logged.
126+
*/
127+
class SloggingMethodSanitizer extends LogInjection::Sanitizer {
128+
SloggingMethodSanitizer() {
129+
this = any(SloggingLoggerMethod m).getMessageArg()
130+
or
131+
this = any(SloggingContextLoggerMethod m).getMessageArg()
132+
or
133+
this = any(SloggingFallbackLoggerMethod m).getMessageArg()
134+
}
135+
}
136+
137+
/**
138+
* Models the SimpleLogger interface methods as safe sinks.
139+
* The SimpleLogger interface is implemented by Logger, ContextLogger, and FallbackLogger,
140+
* all of which sanitize log messages.
141+
*/
142+
class SloggingSimpleLoggerCall extends DataFlow::CallNode {
143+
SloggingSimpleLoggerCall() {
144+
exists(Method m |
145+
// Match any method call on a type that implements SimpleLogger
146+
// where the receiver type is from the slogging package
147+
m.getReceiverType().getUnderlyingType().(PointerType).getBaseType().hasQualifiedName(sloggingPackage(), _) and
148+
m.getName() in ["Debug", "Info", "Warn", "Error"] and
149+
this = m.getACall()
150+
)
151+
}
152+
}

‎.github/codeql/custom-queries/qlpack.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# TMI Custom CodeQL Query Pack
2+
# This pack extends the default Go security queries with TMI-specific models
3+
#
4+
# The slogging package sanitizes all log messages to prevent log injection (CWE-117).
5+
# This pack teaches CodeQL that SanitizeLogMessage() is a sanitizer, preventing
6+
# false positives when user input flows through the slogging logging methods.
7+
8+
name: tmi/custom-queries
9+
version: 1.0.0
10+
libraryPathDependencies:
11+
- codeql/go-all

‎.version‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
22
"major": 0,
3-
"minor": 266,
4-
"patch": 12
3+
"minor": 269,
4+
"patch": 0
55
}

‎api/version.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ var (
2727
// Major version number
2828
VersionMajor = "0"
2929
// Minor version number
30-
VersionMinor = "266"
30+
VersionMinor = "269"
3131
// Patch version number
32-
VersionPatch = "12"
32+
VersionPatch = "0"
3333
// GitCommit is the git commit hash from build
3434
GitCommit = "development"
3535
// BuildDate is the build timestamp

0 commit comments

Comments
 (0)