From f67aa8b9b3cefddddf477850b8d8bffd3c8c26e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Anic=CC=81?= Date: Fri, 23 Feb 2024 21:57:15 +0100 Subject: [PATCH 1/2] std.tar fix parsing mode field in tar header Found by fuzzing. Previous numeric function assumed that is is getting buffer of size 12, mode is size 8. Fuzzing found overflow. Fixing and adding test cases. --- lib/std/tar.zig | 93 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 75 insertions(+), 18 deletions(-) diff --git a/lib/std/tar.zig b/lib/std/tar.zig index 9ef2e1fbb2..439a513161 100644 --- a/lib/std/tar.zig +++ b/lib/std/tar.zig @@ -140,11 +140,25 @@ pub const Header = struct { } pub fn mode(header: Header) !u32 { - return @intCast(try header.numeric(100, 8)); + return @intCast(try header.octal(100, 8)); } pub fn size(header: Header) !u64 { - return header.numeric(124, 12); + const start = 124; + const len = 12; + const raw = header.bytes[start..][0..len]; + // If the leading byte is 0xff (255), all the bytes of the field + // (including the leading byte) are concatenated in big-endian order, + // with the result being a negative number expressed in two’s + // complement form. + if (raw[0] == 0xff) return error.TarNumericValueNegative; + // If the leading byte is 0x80 (128), the non-leading bytes of the + // field are concatenated in big-endian order. + if (raw[0] == 0x80) { + if (raw[1] + raw[2] + raw[3] != 0) return error.TarNumericValueTooBig; + return std.mem.readInt(u64, raw[4..12], .big); + } + return try header.octal(start, len); } pub fn chksum(header: Header) !u64 { @@ -170,22 +184,6 @@ pub const Header = struct { return nullStr(header.bytes[start .. start + len]); } - fn numeric(header: Header, start: usize, len: usize) !u64 { - const raw = header.bytes[start..][0..len]; - // If the leading byte is 0xff (255), all the bytes of the field - // (including the leading byte) are concatenated in big-endian order, - // with the result being a negative number expressed in two’s - // complement form. - if (raw[0] == 0xff) return error.TarNumericValueNegative; - // If the leading byte is 0x80 (128), the non-leading bytes of the - // field are concatenated in big-endian order. - if (raw[0] == 0x80) { - if (raw[1] + raw[2] + raw[3] != 0) return error.TarNumericValueTooBig; - return std.mem.readInt(u64, raw[4..12], .big); - } - return try header.octal(start, len); - } - fn octal(header: Header, start: usize, len: usize) !u64 { const raw = header.bytes[start..][0..len]; // Zero-filled octal number in ASCII. Each numeric field of width w @@ -756,3 +754,62 @@ test "tar PaxIterator" { test { _ = @import("tar/test.zig"); } + +test "tar header parse size" { + const cases = [_]struct { + in: []const u8, + want: u64 = 0, + err: ?anyerror = null, + }{ + // Test base-256 (binary) encoded values. + .{ .in = "", .want = 0 }, + .{ .in = "\x80", .want = 0 }, + .{ .in = "\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01", .want = 1 }, + .{ .in = "\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x02", .want = 0x0102 }, + .{ .in = "\x80\x00\x00\x00\x01\x02\x03\x04\x05\x06\x07\x08", .want = 0x0102030405060708 }, + .{ .in = "\x80\x00\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09", .err = error.TarNumericValueTooBig }, + .{ .in = "\x80\x00\x00\x00\x07\x76\xa2\x22\xeb\x8a\x72\x61", .want = 537795476381659745 }, + + // // Test base-8 (octal) encoded values. + .{ .in = "00000000227\x00", .want = 0o227 }, + .{ .in = " 000000227\x00", .want = 0o227 }, + .{ .in = "00000000228\x00", .err = error.TarHeader }, + .{ .in = "11111111111\x00", .want = 0o11111111111 }, + }; + + for (cases) |case| { + var bytes = [_]u8{0} ** Header.SIZE; + @memcpy(bytes[124 .. 124 + case.in.len], case.in); + var header = Header{ .bytes = &bytes }; + if (case.err) |err| { + try std.testing.expectError(err, header.size()); + } else { + try std.testing.expectEqual(case.want, try header.size()); + } + } +} + +test "tar header parse mode" { + const cases = [_]struct { + in: []const u8, + want: u64 = 0, + err: ?anyerror = null, + }{ + .{ .in = "0000644\x00", .want = 0o644 }, + .{ .in = "0000777\x00", .want = 0o777 }, + .{ .in = "7777777\x00", .want = 0o7777777 }, + .{ .in = "7777778\x00", .err = error.TarHeader }, + .{ .in = "77777777", .want = 0o77777777 }, + .{ .in = "777777777777", .want = 0o77777777 }, + }; + for (cases) |case| { + var bytes = [_]u8{0} ** Header.SIZE; + @memcpy(bytes[100 .. 100 + case.in.len], case.in); + var header = Header{ .bytes = &bytes }; + if (case.err) |err| { + try std.testing.expectError(err, header.mode()); + } else { + try std.testing.expectEqual(case.want, try header.mode()); + } + } +} From 0a86b117bf0a29b4996592d6ad29c46833ae44c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Anic=CC=81?= Date: Fri, 23 Feb 2024 21:57:40 +0100 Subject: [PATCH 2/2] std.tar fix integer overflow in header size parse Found by fuzzing. Fixing code and adding test. --- lib/std/tar.zig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/std/tar.zig b/lib/std/tar.zig index 439a513161..6c67600731 100644 --- a/lib/std/tar.zig +++ b/lib/std/tar.zig @@ -155,7 +155,7 @@ pub const Header = struct { // If the leading byte is 0x80 (128), the non-leading bytes of the // field are concatenated in big-endian order. if (raw[0] == 0x80) { - if (raw[1] + raw[2] + raw[3] != 0) return error.TarNumericValueTooBig; + if (raw[1] != 0 or raw[2] != 0 or raw[3] != 0) return error.TarNumericValueTooBig; return std.mem.readInt(u64, raw[4..12], .big); } return try header.octal(start, len); @@ -769,6 +769,7 @@ test "tar header parse size" { .{ .in = "\x80\x00\x00\x00\x01\x02\x03\x04\x05\x06\x07\x08", .want = 0x0102030405060708 }, .{ .in = "\x80\x00\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09", .err = error.TarNumericValueTooBig }, .{ .in = "\x80\x00\x00\x00\x07\x76\xa2\x22\xeb\x8a\x72\x61", .want = 537795476381659745 }, + .{ .in = "\x80\x80\x80\x00\x01\x02\x03\x04\x05\x06\x07\x08", .err = error.TarNumericValueTooBig }, // // Test base-8 (octal) encoded values. .{ .in = "00000000227\x00", .want = 0o227 },