Skip to content

Commit 9bf7dfb

Browse files
authored
Merge pull request #436 from DataDog/neal.powell/CLOUDS-7101/CLOUDS-7233/CLOUDS-7238/fix.memory.corruption.from.internal.buffer
Fix Data Corruption From Internal Buffer Aliasing
2 parents 750397f + b417c92 commit 9bf7dfb

File tree

3 files changed

+38
-1
lines changed

3 files changed

+38
-1
lines changed

forwarder/internal/logs/fixtures/large_logs_buffer_test.json

Lines changed: 2 additions & 0 deletions
Large diffs are not rendered by default.

forwarder/internal/logs/models.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func NewLog(logBytes []byte, blob storage.Blob, scrubber Scrubber, originalSize
8989
blobNameResourceId := blob.ResourceId()
9090
currLog.blobResourceId = blobNameResourceId
9191
currLog.byteSize = originalSize
92-
currLog.raw = logBytes
92+
currLog.raw = slices.Clone(logBytes)
9393
currLog.Container = blob.Container.Name
9494
currLog.Blob = blob.Name
9595

forwarder/internal/logs/parse_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ var (
6565

6666
//go:embed fixtures/logs_with_level_as_int_or_string.json
6767
logsWithLevelAsIntOrStringData []byte
68+
69+
//go:embed fixtures/large_logs_buffer_test.json
70+
largeLogsBufferTestData []byte
6871
)
6972

7073
func TestParseLogs(t *testing.T) {
@@ -334,3 +337,35 @@ func TestParseActiveDirectoryLogs(t *testing.T) {
334337
})
335338
}
336339
}
340+
341+
// Regression [CLOUDS-7233]: Because of a shared reference to the internal scanner buffer,
342+
// memory corruption would trigger under a particular set of circumstances
343+
func TestParseLargeLogsBufferReuse(t *testing.T) {
344+
t.Parallel()
345+
346+
// GIVEN: The set of circumstances triggering the bug:
347+
// - logs large enough to force buffer overwriting
348+
// - pii scrubber rules are not set
349+
reader := bytes.NewReader(largeLogsBufferTestData)
350+
closer := io.NopCloser(reader)
351+
blob := newBlob("/SUBSCRIPTIONS/TEST-SUB/RESOURCEGROUPS/TEST-RG/PROVIDERS/MICROSOFT.TEST/TEST", "insights-logs-test")
352+
scrubber := logs.NewPiiScrubber(nil)
353+
354+
// WHEN: Parsed via logs.Parse()
355+
parsedLogsIter, _, err := logs.Parse(closer, blob, scrubber)
356+
require.NoError(t, err)
357+
358+
var collectedLogs []*logs.Log
359+
for response := range parsedLogsIter {
360+
require.NoError(t, response.Err)
361+
collectedLogs = append(collectedLogs, response.ParsedLog)
362+
}
363+
364+
// THEN: Make sure logs aren't corrupted/malformed
365+
firstLogContent := string(collectedLogs[0].Content)
366+
assert.Contains(t, firstLogContent, "FIRST_LOG_MARKER", "First log Content should contain FIRST_LOG_MARKER")
367+
assert.NotContains(t, firstLogContent, "SECOND_LOG_MARKER", "First log Content should NOT contain SECOND_LOG_MARKER (buffer reuse corruption)")
368+
369+
secondLogContent := string(collectedLogs[1].Content)
370+
assert.Contains(t, secondLogContent, "SECOND_LOG_MARKER", "Second log Content should contain SECOND_LOG_MARKER")
371+
}

0 commit comments

Comments
 (0)