Enforce tar entry path normalization in pub_package_reader

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<TarArchive> scan(
     String path, {
     int? maxFileCount,
     int? maxTotalLengthBytes,
   }) async {
     final names = <String>{};
+    final normalizedNames = <String>{};
     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<String, String> _normalizeNames(List<String> 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);

pkg/pub_package_reader/test/_tar_writer.dart

@@ -13,6 +13,8 @@ Future<void> writeTarGzFile(
   File file, {
   Map<String, String>? textFiles,
   Map<String, String>? symlinks,
+  List<String>? directories,
+  List<TarEntry>? rawEntries,
 }) async {
   await () async* {
         if (textFiles != null) {
@@ -39,6 +41,23 @@ Future<void> 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<TarEntry>()
       .transform(tarWriter)

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<TarException>().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<TarException>().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`.)',
+        );
+      });
+    });
   });
 }