Skip to content

Commit 8709b7d

Browse files
author
Scott Bommarito
authored
Unknown CVEs should be created when the deprecation form is submitted (#6943)
1 parent 1336379 commit 8709b7d

14 files changed

Lines changed: 503 additions & 129 deletions

src/NuGet.Services.Entities/Cve.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,18 +46,17 @@ public Cve()
4646
/// The description field is intentionally truncated to maximum 300 characters.
4747
/// </remarks>
4848
[MaxLength(300)]
49-
[Required]
5049
public string Description { get; set; }
5150

5251
/// <summary>
5352
/// Gets or sets the last-modified date for the entity.
5453
/// </summary>
55-
public DateTime LastModifiedDate { get; set; }
54+
public DateTime? LastModifiedDate { get; set; }
5655

5756
/// <summary>
5857
/// Gets or sets the date this CVE entity was first published.
5958
/// </summary>
60-
public DateTime PublishedDate { get; set; }
59+
public DateTime? PublishedDate { get; set; }
6160

6261
/// <summary>
6362
/// Gets or sets whether the <see cref="Cve"/> is publicly listed.

src/NuGet.Services.Entities/CveStatus.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ public enum CveStatus
7272
/// Due to a CNA error, the CVE candidate was also originally assigned to another issue.
7373
/// The CVE description will provide details about which other CVEs to refer too.
7474
/// </summary>
75-
Split = 10
75+
Split = 10,
76+
77+
/// <summary>
78+
/// A value of Unknown refers to an entity that was entered by a user of NuGet.org.
79+
/// If the CVE exists, it will be updated with the full data when it is imported.
80+
/// </summary>
81+
Unknown = 11
7682
}
7783
}

src/NuGetGallery.Core/Entities/EntitiesContext.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,8 +382,7 @@ protected override void OnModelCreating(DbModelBuilder modelBuilder)
382382

383383
modelBuilder.Entity<Cve>()
384384
.Property(e => e.Description)
385-
.HasMaxLength(300)
386-
.IsRequired();
385+
.HasMaxLength(300);
387386

388387
modelBuilder.Entity<Cve>()
389388
.Property(v => v.CvssRating)

src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs

Lines changed: 100 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Linq;
77
using System.Net;
8+
using System.Text.RegularExpressions;
89
using System.Threading.Tasks;
910
using System.Web.Mvc;
1011
using NuGet.Services.Entities;
@@ -16,6 +17,15 @@ namespace NuGetGallery
1617
public partial class ManageDeprecationJsonApiController
1718
: AppController
1819
{
20+
private static readonly TimeSpan RegexTimeout = TimeSpan.FromMinutes(1);
21+
22+
public const string CveIdRegexYearGroupName = "year";
23+
public const string CveIdRegexPattern = @"CVE-(?<" + CveIdRegexYearGroupName + @">\d{4})-\d{4,}";
24+
private static readonly Regex CveIdRegex = GetRegexFromPattern(CveIdRegexPattern);
25+
26+
public const string CweIdRegexPattern = @"CWE-\d+";
27+
private static readonly Regex CweIdRegex = GetRegexFromPattern(CweIdRegexPattern);
28+
1929
private readonly IVulnerabilityAutocompleteService _vulnerabilityAutocompleteService;
2030
private readonly IPackageService _packageService;
2131
private readonly IPackageDeprecationService _deprecationService;
@@ -109,6 +119,28 @@ public virtual async Task<JsonResult> Deprecate(
109119
return DeprecateErrorResponse(HttpStatusCode.BadRequest, Strings.DeprecatePackage_NoVersions);
110120
}
111121

122+
JsonResult vulnerabilityDetailIdsErrorResult;
123+
124+
cveIds = cveIds ?? Enumerable.Empty<string>();
125+
if (!TryVerifyVulnerabilityDetailIds(
126+
cveIds,
127+
IsValidCveId,
128+
Strings.DeprecatePackage_InvalidCve,
129+
out vulnerabilityDetailIdsErrorResult))
130+
{
131+
return vulnerabilityDetailIdsErrorResult;
132+
}
133+
134+
cweIds = cweIds ?? Enumerable.Empty<string>();
135+
if (!TryVerifyVulnerabilityDetailIds(
136+
cweIds,
137+
IsValidCweId,
138+
Strings.DeprecatePackage_InvalidCwe,
139+
out vulnerabilityDetailIdsErrorResult))
140+
{
141+
return vulnerabilityDetailIdsErrorResult;
142+
}
143+
112144
if (cvssRating.HasValue && (cvssRating < 0 || cvssRating > 10))
113145
{
114146
return DeprecateErrorResponse(HttpStatusCode.BadRequest, Strings.DeprecatePackage_InvalidCvss);
@@ -180,18 +212,16 @@ public virtual async Task<JsonResult> Deprecate(
180212
}
181213
}
182214

183-
cveIds = cveIds ?? Enumerable.Empty<string>();
184-
var cves = _deprecationService.GetCvesById(cveIds);
185-
if (cveIds.Count() != cves.Count)
215+
var cves = await _deprecationService.GetOrCreateCvesByIdAsync(cveIds, commitChanges: false);
216+
217+
IReadOnlyCollection<Cwe> cwes;
218+
try
186219
{
187-
return DeprecateErrorResponse(HttpStatusCode.NotFound, Strings.DeprecatePackage_MissingCve);
220+
cwes = _deprecationService.GetCwesById(cweIds);
188221
}
189-
190-
cweIds = cweIds ?? Enumerable.Empty<string>();
191-
var cwes = _deprecationService.GetCwesById(cweIds);
192-
if (cweIds.Count() != cwes.Count)
222+
catch (ArgumentException)
193223
{
194-
return DeprecateErrorResponse(HttpStatusCode.NotFound, Strings.DeprecatePackage_MissingCwe);
224+
return DeprecateErrorResponse(HttpStatusCode.NotFound, Strings.DeprecatePackage_CweMissing);
195225
}
196226

197227
var status = PackageDeprecationStatus.NotDeprecated;
@@ -228,5 +258,66 @@ private JsonResult DeprecateErrorResponse(HttpStatusCode code, string error)
228258
{
229259
return Json(code, new { error });
230260
}
261+
262+
/// <summary>
263+
/// Verifies IDs in the list match <paramref name="regex"/>.
264+
/// If they don't, returns <c>false</c> and sets <paramref name="result"/> to the expected <see cref="JsonResult"/>.
265+
/// Otherwise, returns <c>true</c>.
266+
/// </summary>
267+
/// <param name="errorString">The error string to use to construct <paramref name="result"/>.</param>
268+
private bool TryVerifyVulnerabilityDetailIds(
269+
IEnumerable<string> ids,
270+
Func<string, bool> isValid,
271+
string errorString,
272+
out JsonResult result)
273+
{
274+
result = null;
275+
string invalidId;
276+
if ((invalidId = ids.FirstOrDefault(c => !isValid(c))) != null)
277+
{
278+
result = DeprecateErrorResponse(
279+
HttpStatusCode.BadRequest,
280+
string.Format(errorString, invalidId));
281+
282+
return false;
283+
}
284+
285+
return true;
286+
}
287+
288+
private bool IsValidCveId(string id)
289+
{
290+
var match = CveIdRegex.Match(id);
291+
if (match.Value == string.Empty)
292+
{
293+
return false;
294+
}
295+
296+
var yearString = match.Groups[CveIdRegexYearGroupName].Value;
297+
if (!int.TryParse(yearString.ToString(), out var year))
298+
{
299+
return false;
300+
}
301+
302+
if (year < 1999 || year > DateTime.UtcNow.Year)
303+
{
304+
return false;
305+
}
306+
307+
return true;
308+
}
309+
310+
private bool IsValidCweId(string id)
311+
{
312+
return CweIdRegex.IsMatch(id);
313+
}
314+
315+
private static Regex GetRegexFromPattern(string pattern)
316+
{
317+
return new Regex(
318+
pattern,
319+
RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.Singleline,
320+
RegexTimeout);
321+
}
231322
}
232323
}

src/NuGetGallery/Migrations/201903020136235_CvesCanBeEmpty.Designer.cs

Lines changed: 29 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
namespace NuGetGallery.Migrations
2+
{
3+
using System;
4+
using System.Data.Entity.Migrations;
5+
6+
public partial class CvesCanBeEmpty : DbMigration
7+
{
8+
public override void Up()
9+
{
10+
AlterColumn("dbo.Cwes", "Name", c => c.String(nullable: false, maxLength: 200));
11+
AlterColumn("dbo.Cwes", "Description", c => c.String(nullable: false, maxLength: 300));
12+
}
13+
14+
public override void Down()
15+
{
16+
AlterColumn("dbo.Cwes", "Description", c => c.String(maxLength: 300));
17+
AlterColumn("dbo.Cwes", "Name", c => c.String(maxLength: 200));
18+
}
19+
}
20+
}

src/NuGetGallery/Migrations/201903020136235_CvesCanBeEmpty.resx

Lines changed: 126 additions & 0 deletions
Large diffs are not rendered by default.

src/NuGetGallery/NuGetGallery.csproj

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,10 @@
223223
<Compile Include="Authentication\Providers\AzureActiveDirectory\AzureActiveDirectoryAuthenticator.cs" />
224224
<Compile Include="Authentication\Providers\AzureActiveDirectory\AzureActiveDirectoryAuthenticatorConfiguration.cs" />
225225
<Compile Include="Controllers\ManageDeprecationJsonApiController.cs" />
226+
<Compile Include="Migrations\201903020136235_CvesCanBeEmpty.cs" />
227+
<Compile Include="Migrations\201903020136235_CvesCanBeEmpty.Designer.cs">
228+
<DependentUpon>201903020136235_CvesCanBeEmpty.cs</DependentUpon>
229+
</Compile>
226230
<Compile Include="Queries\AutocompleteCveIdQueryResults.cs" />
227231
<Compile Include="Queries\AutocompleteCweIdQueryResults.cs" />
228232
<Compile Include="Queries\AutocompleteCveIdQueryResult.cs" />
@@ -1701,6 +1705,9 @@
17011705
<EmbeddedResource Include="Migrations\201902061444243_AddVulnerabilityEntities.resx">
17021706
<DependentUpon>201902061444243_AddVulnerabilityEntities.cs</DependentUpon>
17031707
</EmbeddedResource>
1708+
<EmbeddedResource Include="Migrations\201903020136235_CvesCanBeEmpty.resx">
1709+
<DependentUpon>201903020136235_CvesCanBeEmpty.cs</DependentUpon>
1710+
</EmbeddedResource>
17041711
<EmbeddedResource Include="OData\QueryAllowed\Data\apiv1packages.json" />
17051712
<EmbeddedResource Include="OData\QueryAllowed\Data\apiv1search.json" />
17061713
<EmbeddedResource Include="OData\QueryAllowed\Data\apiv2getupdates.json" />

src/NuGetGallery/Services/IPackageDeprecationService.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ Task UpdateDeprecation(
2929

3030
/// <summary>
3131
/// Fetches all <see cref="Cve"/>s with a <see cref="Cve.CveId"/> contained in <paramref name="ids"/>.
32+
/// If an ID does not have an associated <see cref="Cve"/>, it creates a dummy entity.
3233
/// </summary>
33-
IReadOnlyCollection<Cve> GetCvesById(IEnumerable<string> ids);
34+
Task<IReadOnlyCollection<Cve>> GetOrCreateCvesByIdAsync(IEnumerable<string> ids, bool commitChanges);
3435

3536
/// <summary>
3637
/// Fetches all <see cref="Cwe"/>s with a <see cref="Cwe.CweId"/> contained in <paramref name="ids"/>.

src/NuGetGallery/Services/PackageDeprecationService.cs

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,40 @@ public async Task UpdateDeprecation(
117117
await _deprecationRepository.CommitChangesAsync();
118118
}
119119

120-
public IReadOnlyCollection<Cve> GetCvesById(IEnumerable<string> ids)
120+
public async Task<IReadOnlyCollection<Cve>> GetOrCreateCvesByIdAsync(IEnumerable<string> ids, bool commitChanges)
121121
{
122122
if (ids == null)
123123
{
124124
throw new ArgumentNullException(nameof(ids));
125125
}
126126

127-
return _cveRepository.GetAll()
127+
var details = _cveRepository.GetAll()
128128
.Where(c => ids.Contains(c.CveId))
129129
.ToList();
130+
131+
var addedDetails = new List<Cve>();
132+
foreach (var missingId in ids.Where(i => !details.Any(c => c.CveId == i)))
133+
{
134+
var detail = new Cve
135+
{
136+
CveId = missingId,
137+
Listed = false,
138+
Status = CveStatus.Unknown
139+
};
140+
addedDetails.Add(detail);
141+
details.Add(detail);
142+
}
143+
144+
if (addedDetails.Any())
145+
{
146+
_cveRepository.InsertOnCommit(addedDetails);
147+
if (commitChanges)
148+
{
149+
await _cveRepository.CommitChangesAsync();
150+
}
151+
}
152+
153+
return details;
130154
}
131155

132156
public IReadOnlyCollection<Cwe> GetCwesById(IEnumerable<string> ids)
@@ -136,9 +160,16 @@ public IReadOnlyCollection<Cwe> GetCwesById(IEnumerable<string> ids)
136160
throw new ArgumentNullException(nameof(ids));
137161
}
138162

139-
return _cweRepository.GetAll()
163+
var cwes = _cweRepository.GetAll()
140164
.Where(c => ids.Contains(c.CweId))
141165
.ToList();
166+
167+
if (ids.Any(i => !cwes.Any(c => i == c.CweId)))
168+
{
169+
throw new ArgumentException("Some IDs do not have a CWE associated with them!", nameof(ids));
170+
}
171+
172+
return cwes;
142173
}
143174
}
144175
}

0 commit comments

Comments
 (0)