Skip to content

Commit e64b064

Browse files
committed
fix use-after-free in ReadWriteLogRecord
Signed-off-by: inge4pres <fgualazzi@gmail.com>
1 parent 15bee3e commit e64b064

File tree

3 files changed

+89
-10
lines changed

3 files changed

+89
-10
lines changed

src/api/logs/logger_provider.zig

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,16 +57,31 @@ pub const ReadWriteLogRecord = struct {
5757
/// Convert to immutable ReadableLogRecord for export
5858
pub fn toReadable(self: *const Self, allocator: std.mem.Allocator) !ReadableLogRecord {
5959
const attrs = try allocator.alloc(Attribute, self.attributes.items.len);
60+
errdefer allocator.free(attrs);
6061
@memcpy(attrs, self.attributes.items);
6162

63+
// Copy severity_text if present
64+
const severity_text = if (self.severity_text) |text|
65+
try allocator.dupe(u8, text)
66+
else
67+
null;
68+
errdefer if (severity_text) |text| allocator.free(text);
69+
70+
// Copy body if present
71+
const body = if (self.body) |b|
72+
try allocator.dupe(u8, b)
73+
else
74+
null;
75+
errdefer if (body) |b| allocator.free(b);
76+
6277
return ReadableLogRecord{
6378
.timestamp = self.timestamp,
6479
.observed_timestamp = self.observed_timestamp,
6580
.trace_id = self.trace_id,
6681
.span_id = self.span_id,
6782
.severity_number = self.severity_number,
68-
.severity_text = self.severity_text,
69-
.body = self.body,
83+
.severity_text = severity_text,
84+
.body = body,
7085
.attributes = attrs,
7186
.resource = self.resource,
7287
.scope = self.scope,
@@ -92,6 +107,8 @@ pub const ReadableLogRecord = struct {
92107

93108
pub fn deinit(self: Self, allocator: std.mem.Allocator) void {
94109
allocator.free(self.attributes);
110+
if (self.severity_text) |text| allocator.free(text);
111+
if (self.body) |b| allocator.free(b);
95112
}
96113
};
97114

src/sdk/logs/log_record_processor.zig

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,6 @@ pub const BatchingLogRecordProcessor = struct {
299299

300300
// Remove exported log records from queue
301301
std.mem.copyForwards(logs.ReadableLogRecord, self.queue.items, self.queue.items[batch_size..]);
302-
303-
// Deinit the exported records before shrinking
304-
for (export_logs) |log_record| {
305-
log_record.deinit(self.allocator);
306-
}
307-
308302
self.queue.shrinkRetainingCapacity(self.queue.items.len - batch_size);
309303

310304
// Export the batch (unlock mutex during export)
@@ -314,6 +308,11 @@ pub const BatchingLogRecordProcessor = struct {
314308
self.exporter.exportLogs(export_logs) catch |err| {
315309
std.log.err("BatchingLogRecordProcessor failed to export log batch: {}", .{err});
316310
};
311+
312+
// Deinit the exported records after export is complete
313+
for (export_logs) |log_record| {
314+
log_record.deinit(self.allocator);
315+
}
317316
}
318317

319318
fn enabled(ctx: *anyopaque, params: EnabledParameters) bool {
@@ -359,14 +358,25 @@ test "SimpleLogRecordProcessor basic functionality" {
359358
const attrs = try self.allocator.alloc(@import("../../attributes.zig").Attribute, log_record.attributes.len);
360359
@memcpy(attrs, log_record.attributes);
361360

361+
// Copy severity_text and body strings since they will be freed after export
362+
const severity_text = if (log_record.severity_text) |text|
363+
try self.allocator.dupe(u8, text)
364+
else
365+
null;
366+
367+
const body = if (log_record.body) |b|
368+
try self.allocator.dupe(u8, b)
369+
else
370+
null;
371+
362372
try self.exported_logs.append(self.allocator, .{
363373
.timestamp = log_record.timestamp,
364374
.observed_timestamp = log_record.observed_timestamp,
365375
.trace_id = log_record.trace_id,
366376
.span_id = log_record.span_id,
367377
.severity_number = log_record.severity_number,
368-
.severity_text = log_record.severity_text,
369-
.body = log_record.body,
378+
.severity_text = severity_text,
379+
.body = body,
370380
.attributes = attrs,
371381
.resource = log_record.resource,
372382
.scope = log_record.scope,

src/sdk/logs/std_log_bridge.zig

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,3 +324,55 @@ test "std_log_bridge severity text mapping" {
324324
try std.testing.expectEqualStrings("info", mapSeverityText(.info));
325325
try std.testing.expectEqualStrings("debug", mapSeverityText(.debug));
326326
}
327+
328+
const sdk = @import("../../sdk.zig");
329+
const InMemoryExporter = @import("../../sdk/logs/exporters/generic.zig").InMemoryExporter;
330+
331+
test "std_log_bridge logFn prints text for body" {
332+
const provider = try LoggerProvider.init(std.testing.allocator, null);
333+
defer provider.deinit();
334+
335+
var buf = try std.ArrayList(u8).initCapacity(std.testing.allocator, 1024);
336+
defer buf.deinit(std.testing.allocator);
337+
338+
var in_mem: InMemoryExporter = .init(buf.writer(std.testing.allocator));
339+
const exporter = in_mem.asLogRecordExporter();
340+
341+
var simple_processor = sdk.logs.SimpleLogRecordProcessor.init(std.testing.allocator, exporter);
342+
const processor = simple_processor.asLogRecordProcessor();
343+
try provider.addLogRecordProcessor(processor);
344+
345+
const cfg = Config{
346+
.provider = provider,
347+
.also_log_to_stderr = false,
348+
};
349+
350+
try configure(cfg);
351+
352+
logFn(std.log.Level.debug, .test_scope, "test text", .{});
353+
354+
const result = try buf.toOwnedSlice(std.testing.allocator);
355+
defer std.testing.allocator.free(result);
356+
try std.testing.expect(std.mem.indexOf(u8, result, "test text") != null);
357+
}
358+
359+
test "std_log_bridge shutdown cleans up state" {
360+
const allocator = std.testing.allocator;
361+
362+
var provider = try LoggerProvider.init(allocator, null);
363+
defer provider.deinit();
364+
365+
try configure(.{
366+
.provider = provider,
367+
.also_log_to_stderr = false,
368+
});
369+
370+
shutdown();
371+
372+
const state = State.get();
373+
state.mutex.lock();
374+
defer state.mutex.unlock();
375+
376+
try std.testing.expect(state.config == null);
377+
try std.testing.expect(state.logger == null);
378+
}

0 commit comments

Comments
 (0)