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] 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()); + } + } +}