diff --git a/pkg/pub_package_reader/lib/src/tar_utils.dart b/pkg/pub_package_reader/lib/src/tar_utils.dart index 8738de73dc..9d6324f161 100644 --- a/pkg/pub_package_reader/lib/src/tar_utils.dart +++ b/pkg/pub_package_reader/lib/src/tar_utils.dart @@ -16,7 +16,7 @@ const _defaultMode = 420; // 644₈ const _executableMask = 0x49; // 001 001 001 /// Abstract interface that once separated process-based tar and package:tar. -class TarArchive { +final class TarArchive { final String _path; /// Maps the normalized names to their original value; @@ -96,12 +96,17 @@ class TarArchive { } /// Creates a new instance by scanning the archive at [path]. + /// + /// Throws [TarException] if the archive contains invalid entry names, non-normalized + /// paths, duplicate entries, symlinks, entries pointing outside of the archive, + /// entries exceeding limits, or entries with invalid mode bits. static Future scan( String path, { int? maxFileCount, int? maxTotalLengthBytes, }) async { final names = {}; + final normalizedNames = {}; final reader = TarReader( File(path).openRead().transform(gzip.decoder), disallowTrailingData: true, @@ -142,18 +147,33 @@ class TarArchive { } final normalizedName = _normalize(entry.name); - if (p.isAbsolute(normalizedName)) { + if (p.posix.isAbsolute(normalizedName)) { throw TarException('Tar entry has absolute name: `${entry.name}`.'); } - if (p.split(normalizedName).contains('..')) { + if (p.posix.split(normalizedName).contains('..')) { throw TarException( 'Tar entry points outside of the archive: `${entry.name}`.', ); } - if (!names.add(entry.name)) { + // In POSIX tar archives, directory entries conventionally end with a + // trailing slash, which `p.posix.normalize` strips. We accept directory + // entries both with and without a trailing slash, while `normalizedNames` + // prevents collisions between the two. + final expectedName = + entry.type == TypeFlag.dir && entry.name.endsWith('/') + ? '$normalizedName/' + : normalizedName; + if (normalizedName == '.' || entry.name != expectedName) { + throw TarException( + 'Tar entry name is not normalized: `${entry.name}`.', + ); + } + + if (!normalizedNames.add(normalizedName)) { throw TarException('Duplicate tar entry: `${entry.name}`.'); } + names.add(entry.name); if (entry.header.linkName != null) { throw TarException('Symlinks not allowed: `${entry.name}`.'); } @@ -171,9 +191,9 @@ Map _normalizeNames(List names) { return files; } -String _normalize(String path) => p.normalize(path).trim(); +String _normalize(String path) => p.posix.normalize(path).trim(); -class TarException implements Exception { +final class TarException implements Exception { final String message; TarException(this.message); diff --git a/pkg/pub_package_reader/test/_tar_writer.dart b/pkg/pub_package_reader/test/_tar_writer.dart index 6c474a16e1..c2c24857ab 100644 --- a/pkg/pub_package_reader/test/_tar_writer.dart +++ b/pkg/pub_package_reader/test/_tar_writer.dart @@ -13,6 +13,8 @@ Future writeTarGzFile( File file, { Map? textFiles, Map? symlinks, + List? directories, + List? rawEntries, }) async { await () async* { if (textFiles != null) { @@ -39,6 +41,23 @@ Future writeTarGzFile( ); } } + if (directories != null) { + for (final d in directories) { + yield TarEntry.data( + TarHeader( + name: d, + typeFlag: TypeFlag.dir, + mode: 493, // 755₈ + ), + Uint8List(0), + ); + } + } + if (rawEntries != null) { + for (final entry in rawEntries) { + yield entry; + } + } }() .cast() .transform(tarWriter) diff --git a/pkg/pub_package_reader/test/file_list_test.dart b/pkg/pub_package_reader/test/file_list_test.dart index 6b41db63b3..6305f6569c 100644 --- a/pkg/pub_package_reader/test/file_list_test.dart +++ b/pkg/pub_package_reader/test/file_list_test.dart @@ -81,6 +81,27 @@ void main() { 'Failed to scan tar archive. (Duplicate tar entry: `README.md`.)', ); }); + + test('duplicate directory with and without trailing slash', () async { + await _withTempDir((tempDir) async { + final file = File(p.join(tempDir, 'x.tar.gz')); + await writeTarGzFile( + file, + directories: ['dir', 'dir/'], + textFiles: {'dir/file.txt': 'content'}, + ); + await expectLater( + TarArchive.scan(file.path), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Duplicate tar entry: `dir/`.'), + ), + ), + ); + }); + }); }); group('tar entry test', () { @@ -132,6 +153,78 @@ void main() { }); } }); + + test('non-normalized path in the tar entry', () async { + final alternatives = [ + './abc', + './pubspec.yaml', + 'abc/./def', + 'abc//def', + 'abc/def/', + 'abc/../abc/def', + 'abc/def ', + ' abc/def', + '.', + './', + ]; + for (final path in alternatives) { + await _withTempDir((tempDir) async { + final file = File(p.join(tempDir, 'x.tar.gz')); + await writeTarGzFile(file, textFiles: {path: 'content'}); + await expectLater( + TarArchive.scan(file.path), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Tar entry name is not normalized: `$path`.'), + ), + ), + ); + }); + } + }); + + test('valid normalized paths and directories', () async { + await _withTempDir((tempDir) async { + final file = File(p.join(tempDir, 'x.tar.gz')); + await writeTarGzFile( + file, + directories: ['dir1/', 'dir2'], + textFiles: { + 'pubspec.yaml': 'name: abc', + 'lib/foo.dart': 'void main() {}', + '.gitignore': 'build/', + }, + ); + final archive = await TarArchive.scan(file.path); + expect( + archive.fileNames, + containsAll([ + 'dir1', + 'dir2', + 'pubspec.yaml', + 'lib/foo.dart', + '.gitignore', + ]), + ); + }); + }); + + test('non-normalized entry in summarizePackageArchive', () async { + await _withTempDir((tempDir) async { + final file = File(p.join(tempDir, 'x.tar.gz')); + await writeTarGzFile( + file, + textFiles: {'./pubspec.yaml': minimalTextFiles['pubspec.yaml']!}, + ); + final summary = await summarizePackageArchive(file.path); + expect( + summary.issues.single.message, + 'Failed to scan tar archive. (Tar entry name is not normalized: `./pubspec.yaml`.)', + ); + }); + }); }); }