Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion go/ql/lib/semmle/go/security/LogInjectionCustomizations.qll
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,33 @@ module LogInjection {
)
}
}
}

/**
* Returns true if `t` is a zap encoder type that is considered safe.
*
* We intentionally whitelist *only* JSONEncoder.
* Other encoders may not escape newline characters and therefore
* must NOT be treated as sanitizers.
*/
private predicate isSafeZapEncoder(Type t) {
exists(Type zapEncoder |
// Matches go.uber.org/zap/zapcore.JSONEncoder
zapEncoder.hasQualifiedName("go.uber.org/zap/zapcore", "JSONEncoder") and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type doesn't exist, at least according to https://pkg.go.dev/go.uber.org/zap/zapcore.

t = zapEncoder
)
}

/**
* Zap encoder sanitizer class.
*
* This extends the Sanitizer class used by the go/log-injection query.
*/
class ZapEncoderSanitizer extends Sanitizer {
ZapEncoderSanitizer() {
exists(Type t |
this.getType() = t and
isSafeZapEncoder(t)
)
}
}
}
20 changes: 20 additions & 0 deletions go/ql/test/query-tests/Security/CWE-117/LogInjection.go
Original file line number Diff line number Diff line change
Expand Up @@ -724,3 +724,23 @@ func handlerGood5(req *http.Request) {
object := req.URL.Query()["username"][0]
log.Printf("found object of type %T.\n", object)
}

// UNSAFE: zap.NewProduction() uses production encoder but is NOT considered a sanitizer by our customization.
func zapUnsafeExample(req *http.Request) {
username := req.URL.Query()["username"][0]

logger, _ := zap.NewProduction()
logger.Info("Login attempt by:", username) // $ hasTaintFlow="username"
}

// GOOD: zap logger using a JSONEncoder that our customization treats as a sanitizer.
func zapSafeExample(req *http.Request) {
username := req.URL.Query()["username"][0]

cfg := zap.NewProductionConfig()
// Explicitly force JSONEncoder so that your sanitizer will match it.
cfg.Encoding = "json"

logger, _ := cfg.Build()
logger.Info("Login attempt", zap.String("username", username)) // NO finding expected
}