amadan

carlos / platform Public

clamd: a limit heuristic is unscanned, and a scan timeout is retried

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 741e5bdc6c0750803bdfe11331a49382930c54c6 parent b6a34b7
2 files changed, +90 −2
  • internal/clamd/clamd.go +27 −2
  • internal/clamd/clamd_test.go +63 −0
diff --git a/internal/clamd/clamd.go b/internal/clamd/clamd.go
index a6e51630c..97c46787a 100644
--- a/internal/clamd/clamd.go
+++ b/internal/clamd/clamd.go
@@ -39,6 +39,7 @@ const (
// because it is larger than StreamMaxLength. This is deliberately NOT
// StatusError: nothing failed, the object simply was not judged, and an
// audit that cannot tell those apart is not worth keeping.
+ // It also covers clamd's limit heuristics (an archive that expands past MaxScanSize, MaxFileSize, MaxFiles or MaxRecursion), which arrive as FOUND when AlertExceedsMax is on.
StatusSizeExceeded Status = "size-exceeded"
// StatusError is the engine reading the stream and failing on it.
StatusError Status = "error"
@@ -77,6 +78,11 @@ const (
// the same order of magnitude and the frame header is 4 bytes, so this
// keeps the framing overhead under a thousandth.
chunkSize = 64 * 1024
+
+ // clamd's names for "I stopped before the end". With AlertExceedsMax yes it
+ // reports them as FOUND, which parseReply must not read as an infection.
+ limitHeuristic = "Heuristics.Limits.Exceeded"
+ limitHeuristicScanTime = "Heuristics.Limits.Exceeded.MaxScanTime"
)
func (t Timeouts) withDefaults() Timeouts {
@@ -265,6 +271,13 @@ func (c *Client) Version(ctx context.Context) (string, error) {
return c.readReply(ctx, conn)
}
+// isLimitHeuristic reports whether sig names a scan limit rather than malware.
+// A prefix on the signature, with the dot, so neither a longer first component
+// nor a real signature carrying the phrase elsewhere can match.
+func isLimitHeuristic(sig string) bool {
+ return sig == limitHeuristic || strings.HasPrefix(sig, limitHeuristic+".")
+}
+
// parseReply maps clamd's one-line answers onto a Status.
//
// The size check comes first and deliberately: the refusal arrives as
@@ -278,8 +291,20 @@ func parseReply(reply string) Result {
case strings.HasSuffix(reply, " OK"):
res.Status = StatusClean
case strings.HasSuffix(reply, " FOUND"):
- res.Status = StatusInfected
- res.Signature = signatureOf(reply)
+ sig := signatureOf(reply)
+ switch {
+ case sig == limitHeuristicScanTime:
+ // Load-dependent, so retried: an error becomes a failed verdict,
+ // which every sweep re-issues.
+ res.Status = StatusError
+ case isLimitHeuristic(sig):
+ // Deterministic for these bytes and this config, and nothing was
+ // judged: the same fact as an object too large to stream.
+ res.Status = StatusSizeExceeded
+ default:
+ res.Status = StatusInfected
+ res.Signature = sig
+ }
default:
res.Status = StatusError
}
diff --git a/internal/clamd/clamd_test.go b/internal/clamd/clamd_test.go
index aa88665e8..f15fc1702 100644
--- a/internal/clamd/clamd_test.go
+++ b/internal/clamd/clamd_test.go
@@ -306,3 +306,66 @@ func TestDialRefusesEmptyAddress(t *testing.T) {
t.Fatalf("err = %v", err)
}
}
+
+// With AlertExceedsMax on, clamd reports an archive it could only partly scan
+// as a FOUND whose signature names the limit. That is not an infection and it
+// is not clean: nothing was judged, so it is the same status as an object too
+// large to stream. Recording it clean would sign a verdict over bytes nobody
+// read; recording it infected would make enforce block large honest archives.
+func TestLimitHeuristicIsSizeExceededNotInfected(t *testing.T) {
+ for _, limit := range []string{"MaxScanSize", "MaxFileSize", "MaxFiles", "MaxRecursion"} {
+ reply := "stream: Heuristics.Limits.Exceeded." + limit + " FOUND\x00"
+ res := replyFixture(t, reply)
+ if res.Status != StatusSizeExceeded {
+ t.Errorf("%s: status = %v, want size-exceeded", limit, res.Status)
+ }
+ if res.Signature != "" {
+ t.Errorf("%s: a limit is not a signature, got %q", limit, res.Signature)
+ }
+ if !strings.Contains(res.Raw, limit) {
+ t.Errorf("%s: the engine's own words did not survive: %q", limit, res.Raw)
+ }
+ }
+}
+
+// Older engines name the limit without saying which one.
+func TestBareLimitHeuristicIsSizeExceeded(t *testing.T) {
+ res := replyFixture(t, "stream: Heuristics.Limits.Exceeded FOUND\x00")
+ if res.Status != StatusSizeExceeded {
+ t.Fatalf("status = %v, want size-exceeded", res.Status)
+ }
+}
+
+// A scan that ran out of TIME depends on load, not on the bytes, so it must be
+// retried. StatusError becomes a failed verdict, which every sweep re-issues;
+// size-exceeded becomes skipped, which is carried forward until the object
+// changes. Filing a timeout under skipped would exclude the object for good.
+func TestScanTimeLimitIsAnErrorSoItIsRetried(t *testing.T) {
+ res := replyFixture(t, "stream: Heuristics.Limits.Exceeded.MaxScanTime FOUND\x00")
+ if res.Status != StatusError {
+ t.Fatalf("status = %v, want error", res.Status)
+ }
+ if !strings.Contains(res.Raw, "MaxScanTime") {
+ t.Fatalf("the engine's own words did not survive: %q", res.Raw)
+ }
+}
+
+// The match is on the SIGNATURE, as a prefix. A real signature that happens to
+// carry the phrase elsewhere in its name is still an infection, and so is one
+// that only begins with something similar.
+func TestLookalikeSignaturesAreStillInfected(t *testing.T) {
+ for _, sig := range []string{
+ "Win.Heuristics.Limits.Exceeded.Fake",
+ "Heuristics.Limits.ExceededButNot",
+ "Heuristics.Phishing.Email.SpoofedDomain",
+ "Eicar-Signature",
+ } {
+ res := replyFixture(t, "stream: "+sig+" FOUND\x00")
+ if res.Status != StatusInfected {
+ t.Errorf("%s: status = %v, want infected", sig, res.Status)
+ }
+ if res.Signature != sig {
+ t.Errorf("signature = %q, want %q", res.Signature, sig)
+ }
+ }
+}