diff --git a/changelog.d/unreleased/3062.security.md b/changelog.d/unreleased/3062.security.md new file mode 100644 index 0000000000..dc08f86c8a --- /dev/null +++ b/changelog.d/unreleased/3062.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3062 +affected: + - src/CodeIndex/Cli/SuggestionStore.cs + - tests/CodeIndex.Tests/SuggestionStoreTests.cs +--- + +## English + +- **Suggestion store reads now cap JSON depth and record count (#3062)** — local suggestion store reads reject overly deep JSON and stores with more than the supported record limit, preserving the rejected file as `.bak` instead of allocating an unbounded in-memory record set. + +## 日本語 + +- **suggestion store の読み込みで JSON depth と record 数を制限するようになりました (#3062)** — ローカル suggestion store の読み込みは深すぎる JSON や対応上限を超える record 数のストアを拒否し、無制限のメモリ上 record 集合を作らず対象ファイルを `.bak` として保存します。 diff --git a/src/CodeIndex/Cli/SuggestionStore.cs b/src/CodeIndex/Cli/SuggestionStore.cs index fccfce7038..9de8c01c19 100644 --- a/src/CodeIndex/Cli/SuggestionStore.cs +++ b/src/CodeIndex/Cli/SuggestionStore.cs @@ -33,8 +33,10 @@ public class SuggestionStore internal const int DefaultMaxAgeDays = 365; internal const int DefaultMaxCount = 5000; internal const int MaxSuggestionStoreBytes = 8 * 1024 * 1024; + internal const int MaxSuggestionStoreJsonDepth = 16; internal const int MaximumMaxAgeDays = 3650; internal const int MaximumMaxCount = 100_000; + internal const int MaxSuggestionStoreRecords = MaximumMaxCount; private const int FuzzyDedupRecentLimit = 100; private const string RedactedAwsAccessKey = "[REDACTED:aws_access_key]"; private const string RedactedBearerToken = "[REDACTED:bearer_token]"; @@ -79,6 +81,7 @@ public class SuggestionStore { PropertyNamingPolicy = JsonNamingPolicy.SnakeCaseLower, PropertyNameCaseInsensitive = true, + MaxDepth = MaxSuggestionStoreJsonDepth, }; static SuggestionStore() @@ -440,32 +443,14 @@ public void MarkSubmitted(string hash, string issueUrl) /// private List ReadUnlocked() { - var ioPath = LongPath.EnsureWindowsPrefix(_filePath); - if (!File.Exists(ioPath)) - return new List(); - - if (new FileInfo(ioPath).Length == 0) - { - PreserveCorruptFile(); - return new List(); - } - - var json = DataDirectorySecurity.ReadTextWithinLimit(ioPath, MaxSuggestionStoreBytes, StreamingReadFileShare); - if (json is null) - { - PreserveCorruptFile(); - return new List(); - } - - if (string.IsNullOrWhiteSpace(json)) + if (!TryReadStoreSnapshot(out var snapshot)) return new List(); try { - var records = JsonSerializer.Deserialize>(json, s_readOptions) - ?? new List(); - NormalizeRecordDefaults(records); - return records; + return ReadFilteredSnapshotAsync(snapshot, _ => true, skip: 0, take: null, normalizeDefaults: true) + .GetAwaiter() + .GetResult(); } catch (JsonException) { @@ -481,15 +466,45 @@ private List ReadUnlocked() private static void NormalizeRecordDefaults(List records) { foreach (var record in records) + NormalizeRecordDefaults(record); + } + + private static void NormalizeRecordDefaults(SuggestionRecord record) + { + NormalizeLegacyFields(record); + if (string.IsNullOrWhiteSpace(record.CreatedByAgent)) + record.CreatedByAgent = "unknown"; + if (string.IsNullOrWhiteSpace(record.SessionId)) + record.SessionId = "unknown"; + if (string.IsNullOrWhiteSpace(record.ClientVersion)) + record.ClientVersion = "unknown"; + } + + private bool TryReadStoreSnapshot(out byte[] snapshot) + { + snapshot = Array.Empty(); + var ioPath = LongPath.EnsureWindowsPrefix(_filePath); + if (!File.Exists(ioPath)) + return false; + + var readSnapshot = DataDirectorySecurity.ReadBytesWithinLimit(ioPath, MaxSuggestionStoreBytes, StreamingReadFileShare); + if (readSnapshot is null) { - NormalizeLegacyFields(record); - if (string.IsNullOrWhiteSpace(record.CreatedByAgent)) - record.CreatedByAgent = "unknown"; - if (string.IsNullOrWhiteSpace(record.SessionId)) - record.SessionId = "unknown"; - if (string.IsNullOrWhiteSpace(record.ClientVersion)) - record.ClientVersion = "unknown"; + PreserveCorruptFile(); + return false; } + + if (readSnapshot.Length == 0) + { + PreserveCorruptFile(); + return false; + } + + if (IsEmptyOrJsonWhitespace(readSnapshot)) + return false; + + snapshot = readSnapshot; + return true; } private static (SuggestionRecord? Record, double? Score) FindDuplicate( @@ -635,29 +650,14 @@ private List ReadFilteredUnlocked( int skip = 0, int? take = null) { - var ioPath = LongPath.EnsureWindowsPrefix(_filePath); - if (!File.Exists(ioPath)) - return new List(); - - var snapshot = DataDirectorySecurity.ReadBytesWithinLimit(ioPath, MaxSuggestionStoreBytes, StreamingReadFileShare); - if (snapshot is null) - { - PreserveCorruptFile(); - return new List(); - } - - if (snapshot.Length == 0) - { - PreserveCorruptFile(); - return new List(); - } - - if (IsEmptyOrJsonWhitespace(snapshot)) + if (!TryReadStoreSnapshot(out var snapshot)) return new List(); try { - return ReadFilteredSnapshotAsync(snapshot, predicate, skip, take).GetAwaiter().GetResult(); + return ReadFilteredSnapshotAsync(snapshot, predicate, skip, take, normalizeDefaults: false) + .GetAwaiter() + .GetResult(); } catch (JsonException) { @@ -698,27 +698,37 @@ private static async Task> ReadFilteredSnapshotAsync( byte[] snapshot, Func predicate, int skip, - int? take) + int? take, + bool normalizeDefaults) { var results = new List(); + var recordsRead = 0; var skipped = 0; await using var stream = new MemoryStream(snapshot, writable: false); await foreach (var record in JsonSerializer.DeserializeAsyncEnumerable(stream, s_readOptions)) { + recordsRead++; + if (recordsRead > MaxSuggestionStoreRecords) + throw new JsonException($"Suggestion store contains more than {MaxSuggestionStoreRecords} records."); + if (record == null || !predicate(record)) continue; + if (normalizeDefaults) + NormalizeRecordDefaults(record); + if (skipped < skip) { skipped++; continue; } - results.Add(record); if (take.HasValue && results.Count >= take.Value) - break; + continue; + + results.Add(record); } return results; diff --git a/tests/CodeIndex.Tests/SuggestionStoreTests.cs b/tests/CodeIndex.Tests/SuggestionStoreTests.cs index c624ef1ad0..40845f3ead 100644 --- a/tests/CodeIndex.Tests/SuggestionStoreTests.cs +++ b/tests/CodeIndex.Tests/SuggestionStoreTests.cs @@ -1,4 +1,5 @@ using System.Globalization; +using System.Text; using CodeIndex.Cli; using CodeIndex.Models; @@ -108,6 +109,58 @@ public void LoadByStatus_OversizedStore_PreservesBackupAndReturnsEmpty() Assert.True(File.Exists(path + ".bak")); } + [Fact] + public void LoadAll_ExcessiveJsonDepth_PreservesBackupAndReturnsEmpty() + { + var path = Path.Combine(_tempDir, "suggestions-codeindex.json"); + File.WriteAllText(path, BuildDeepSuggestionStoreJson(SuggestionStore.MaxSuggestionStoreJsonDepth + 1)); + + var records = _store.LoadAll(); + + Assert.Empty(records); + Assert.False(File.Exists(path)); + Assert.True(File.Exists(path + ".bak")); + } + + [Fact] + public void LoadAll_TooManyRecords_PreservesBackupAndReturnsEmpty() + { + var path = Path.Combine(_tempDir, "suggestions-codeindex.json"); + WriteEmptyRecordStore(path, SuggestionStore.MaxSuggestionStoreRecords + 1); + + var records = _store.LoadAll(); + + Assert.Empty(records); + Assert.False(File.Exists(path)); + Assert.True(File.Exists(path + ".bak")); + } + + [Fact] + public void LoadByStatus_TooManyRecordsBeforeFilter_PreservesBackupAndReturnsEmpty() + { + var path = Path.Combine(_tempDir, "suggestions-codeindex.json"); + WriteEmptyRecordStore(path, SuggestionStore.MaxSuggestionStoreRecords + 1); + + var records = _store.LoadByStatus(SuggestionStatus.SubmittedPendingTriage); + + Assert.Empty(records); + Assert.False(File.Exists(path)); + Assert.True(File.Exists(path + ".bak")); + } + + [Fact] + public void LoadPage_TooManyRecordsAfterPage_PreservesBackupAndReturnsEmpty() + { + var path = Path.Combine(_tempDir, "suggestions-codeindex.json"); + WriteEmptyRecordStore(path, SuggestionStore.MaxSuggestionStoreRecords + 1); + + var records = _store.Load(skip: 0, take: 1); + + Assert.Empty(records); + Assert.False(File.Exists(path)); + Assert.True(File.Exists(path + ".bak")); + } + // --- TryAdd tests / TryAdd テスト --- [Fact] @@ -1027,6 +1080,34 @@ public void TryAdd_MoveFailure_DoesNotLeaveOrphanTmpFile() // --- Helpers / ヘルパー --- + private static string BuildDeepSuggestionStoreJson(int nestedObjectCount) + { + var builder = new StringBuilder(); + builder.Append("[{\"category\":\"other\",\"description\":\"Deep suggestion\",\"hash\":\"deep\",\"ignored\":"); + for (var i = 0; i < nestedObjectCount; i++) + builder.Append("{\"x\":"); + builder.Append("\"value\""); + for (var i = 0; i < nestedObjectCount; i++) + builder.Append('}'); + builder.Append("}]"); + return builder.ToString(); + } + + private static void WriteEmptyRecordStore(string path, int count) + { + var builder = new StringBuilder(capacity: (count * 3) + 2); + builder.Append('['); + for (var i = 0; i < count; i++) + { + if (i > 0) + builder.Append(','); + builder.Append("{}"); + } + + builder.Append(']'); + File.WriteAllText(path, builder.ToString()); + } + private static SuggestionRecord MakeRecord(string category, string? language, string description) { return new SuggestionRecord