Skip to content

Commit ff06245

Browse files
openbulletopenbullet
committed
Security hardening
1 parent dcf4049 commit ff06245

6 files changed

Lines changed: 232 additions & 6 deletions

File tree

‎OpenBullet2.Web.Tests/Integration/JobIntegrationTests.cs‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
using RuriLib.Models.Hits;
2626
using RuriLib.Models.Jobs;
2727
using RuriLib.Models.Jobs.StartConditions;
28+
using RuriLib.Models.Proxies;
2829
using RuriLib.Services;
2930
using Xunit;
3031

@@ -1096,6 +1097,65 @@ public async Task CreateMultiRunJob_Guest_Success()
10961097
Assert.Equal(guest.Id, resultJob.OwnerId);
10971098
}
10981099

1100+
[Fact]
1101+
public async Task CreateMultiRunJob_Guest_ScriptFileProxySource_Forbidden()
1102+
{
1103+
// Arrange
1104+
using var client = Factory.CreateClient();
1105+
var configRepository = GetRequiredService<IConfigRepository>();
1106+
var config = new Config
1107+
{
1108+
Id = Guid.NewGuid().ToString(),
1109+
Metadata = new ConfigMetadata { Name = "Test Config" }
1110+
};
1111+
await configRepository.SaveAsync(config);
1112+
1113+
var dbContext = GetRequiredService<ApplicationDbContext>();
1114+
var guest = new GuestEntity { Username = "guest", AccessExpiration = DateTime.MaxValue };
1115+
dbContext.Guests.Add(guest);
1116+
await dbContext.SaveChangesAsync(TestCancellationToken);
1117+
1118+
RequireLogin();
1119+
ImpersonateGuest(client, guest);
1120+
1121+
var dto = new CreateMultiRunJobDto
1122+
{
1123+
Name = "Test MRJ",
1124+
ConfigId = config.Id,
1125+
Bots = 10,
1126+
ProxyMode = JobProxyMode.On,
1127+
DataPool = JsonSerializer.SerializeToElement(new InfiniteDataPoolOptionsDto
1128+
{
1129+
PolyTypeName = "infiniteDataPool"
1130+
}, JsonSerializerOptions),
1131+
HitOutputs = [JsonSerializer.SerializeToElement(new DatabaseHitOutputOptionsDto
1132+
{
1133+
PolyTypeName = "databaseHitOutput"
1134+
}, JsonSerializerOptions)],
1135+
ProxySources = [JsonSerializer.SerializeToElement(new FileProxySourceOptionsDto
1136+
{
1137+
FileName = Path.Combine(UserDataFolder, "Wordlists", "guest-proxies.ps1"),
1138+
DefaultType = ProxyType.Http,
1139+
PolyTypeName = "fileProxySource"
1140+
}, JsonSerializerOptions)],
1141+
StartCondition = JsonSerializer.SerializeToElement(new RelativeTimeStartConditionDto
1142+
{
1143+
StartAfter = TimeSpan.FromSeconds(1),
1144+
PolyTypeName = "relativeTimeStartCondition"
1145+
}, JsonSerializerOptions)
1146+
};
1147+
1148+
// Act
1149+
var result = await PostJsonAsync<MultiRunJobDto>(
1150+
client, "/api/v1/job/multi-run", dto);
1151+
1152+
// Assert
1153+
Assert.False(result.IsSuccess);
1154+
Assert.Equal(HttpStatusCode.Forbidden, result.Error.Response.StatusCode);
1155+
Assert.Equal(ErrorCode.ScriptFileNotAllowed, result.Error.Content!.ErrorCode);
1156+
Assert.Empty(await dbContext.Jobs.ToListAsync(TestCancellationToken));
1157+
}
1158+
10991159
// Admin can create a proxy check job
11001160
[Fact]
11011161
public async Task CreateProxyCheckJob_Admin_Success()
@@ -1465,6 +1525,85 @@ public async Task UpdateMultiRunJob_Guest_Owned_Success()
14651525
Assert.Equal(dto.Name, mrJob.Name);
14661526
}
14671527

1528+
[Fact]
1529+
public async Task UpdateMultiRunJob_Guest_ScriptFileProxySource_Forbidden()
1530+
{
1531+
// Arrange
1532+
using var client = Factory.CreateClient();
1533+
var jobManager = GetRequiredService<JobManagerService>();
1534+
var dbContext = GetRequiredService<ApplicationDbContext>();
1535+
var guest = new GuestEntity { Id = 1, Username = "guest", AccessExpiration = DateTime.MaxValue };
1536+
dbContext.Guests.Add(guest);
1537+
var mrJob = CreateMultiRunJob();
1538+
mrJob.Name = "Test MRJ";
1539+
mrJob.Id = 1;
1540+
mrJob.OwnerId = guest.Id;
1541+
var jobEntity = CreateMultiRunJobEntity(mrJob);
1542+
jobEntity.Id = mrJob.Id;
1543+
jobEntity.Owner = guest;
1544+
dbContext.Jobs.Add(jobEntity);
1545+
jobManager.AddJob(mrJob);
1546+
await dbContext.SaveChangesAsync(TestCancellationToken);
1547+
1548+
RequireLogin();
1549+
ImpersonateGuest(client, guest);
1550+
1551+
var configRepository = GetRequiredService<IConfigRepository>();
1552+
var config = new Config
1553+
{
1554+
Id = Guid.NewGuid().ToString(),
1555+
Metadata = new ConfigMetadata { Name = "Test Config" }
1556+
};
1557+
await configRepository.SaveAsync(config);
1558+
1559+
var dto = new UpdateMultiRunJobDto
1560+
{
1561+
Id = mrJob.Id,
1562+
Name = "Test MRJ2",
1563+
ConfigId = config.Id,
1564+
Bots = 10,
1565+
Skip = 5,
1566+
ProxyMode = JobProxyMode.On,
1567+
DataPool = JsonSerializer.SerializeToElement(new InfiniteDataPoolOptionsDto
1568+
{
1569+
PolyTypeName = "infiniteDataPool"
1570+
}, JsonSerializerOptions),
1571+
HitOutputs =
1572+
[
1573+
JsonSerializer.SerializeToElement(new DatabaseHitOutputOptionsDto
1574+
{
1575+
PolyTypeName = "databaseHitOutput"
1576+
}, JsonSerializerOptions)
1577+
],
1578+
ProxySources =
1579+
[
1580+
JsonSerializer.SerializeToElement(new FileProxySourceOptionsDto
1581+
{
1582+
FileName = Path.Combine(UserDataFolder, "Wordlists", "guest-proxies.sh"),
1583+
DefaultType = ProxyType.Http,
1584+
PolyTypeName = "fileProxySource"
1585+
}, JsonSerializerOptions)
1586+
],
1587+
StartCondition = JsonSerializer.SerializeToElement(new RelativeTimeStartConditionDto
1588+
{
1589+
StartAfter = TimeSpan.FromSeconds(1),
1590+
PolyTypeName = "relativeTimeStartCondition"
1591+
}, JsonSerializerOptions)
1592+
};
1593+
1594+
// Act
1595+
var response = await PutJsonAsync<MultiRunJobDto>(
1596+
client, "/api/v1/job/multi-run", dto);
1597+
1598+
// Assert
1599+
Assert.False(response.IsSuccess);
1600+
Assert.Equal(HttpStatusCode.Forbidden, response.Error.Response.StatusCode);
1601+
Assert.Equal(ErrorCode.ScriptFileNotAllowed, response.Error.Content!.ErrorCode);
1602+
1603+
mrJob = jobManager.Jobs.OfType<MultiRunJob>().Single();
1604+
Assert.Equal("Test MRJ", mrJob.Name);
1605+
}
1606+
14681607
// Admin can update a proxy check job
14691608
[Fact]
14701609
public async Task UpdateProxyCheckJob_Admin_Success()

‎OpenBullet2.Web.Tests/Integration/WordlistIntegrationTests.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
using System.Net;
2+
using System.Net.Http.Json;
23
using System.Text.Json;
34
using Microsoft.EntityFrameworkCore;
45
using OpenBullet2.Core;
56
using OpenBullet2.Core.Entities;
67
using OpenBullet2.Web.Dtos.Common;
78
using OpenBullet2.Web.Dtos.Wordlist;
89
using OpenBullet2.Web.Exceptions;
10+
using OpenBullet2.Web.Models.Errors;
911
using OpenBullet2.Web.Tests.Extensions;
1012
using RuriLib.Extensions;
1113
using Xunit;
@@ -555,6 +557,31 @@ public async Task UploadWordlistFile_Admin_Success()
555557
Path.Combine(UserDataFolder, "Wordlists")));
556558
}
557559

560+
[Fact]
561+
public async Task UploadWordlistFile_ScriptExtension_Forbidden()
562+
{
563+
// Arrange
564+
using var client = Factory.CreateClient();
565+
await using var stream = new MemoryStream("echo test"u8.ToArray());
566+
var content = new MultipartFormDataContent
567+
{
568+
{ new StreamContent(stream), "file", "test.sh" }
569+
};
570+
571+
// Act
572+
var response = await client.PostAsync(
573+
"/api/v1/wordlist/upload", content, TestCancellationToken);
574+
575+
// Assert
576+
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
577+
578+
var dto = await response.Content.ReadFromJsonAsync<ApiError>(
579+
JsonSerializerOptions, TestCancellationToken);
580+
581+
Assert.NotNull(dto);
582+
Assert.Equal(ErrorCode.ScriptFileNotAllowed, dto.ErrorCode);
583+
}
584+
558585
[Fact]
559586
public async Task DeleteWordlist_Admin_WithoutFile_Success()
560587
{

‎OpenBullet2.Web/Controllers/JobController.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using Microsoft.EntityFrameworkCore;
44
using Newtonsoft.Json;
55
using OpenBullet2.Core.Entities;
6+
using OpenBullet2.Core.Models.Data;
67
using OpenBullet2.Core.Models.Hits;
78
using OpenBullet2.Core.Models.Jobs;
89
using OpenBullet2.Core.Models.Proxies;
@@ -19,6 +20,7 @@
1920
using OpenBullet2.Web.Interfaces;
2021
using OpenBullet2.Web.Models.Identity;
2122
using OpenBullet2.Web.Utils;
23+
using RuriLib.Extensions;
2224
using RuriLib.Models.Data.DataPools;
2325
using RuriLib.Models.Hits.HitOutputs;
2426
using RuriLib.Models.Jobs;
@@ -46,6 +48,7 @@ public class JobController(IJobRepository jobRepo, ILogger<JobController> logger
4648
private readonly IObjectMapper _mapper = mapper;
4749
private readonly IProxyGroupRepository _proxyGroupRepo = proxyGroupRepo;
4850
private readonly IRecordRepository _recordRepo = recordRepo;
51+
private static readonly string[] ScriptExtensions = [".bat", ".ps1", ".sh"];
4952

5053
/// <summary>
5154
/// Get overview information about all jobs.
@@ -243,6 +246,8 @@ public async Task<ActionResult<MultiRunJobDto>> CreateMultiRunJob(
243246
var apiUser = HttpContext.GetApiUser();
244247
var jobOptions = _mapper.Map<MultiRunJobOptions>(dto);
245248

249+
ValidateGuestLocalFileUsage(apiUser, jobOptions);
250+
246251
var jsonSettings = new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.Auto };
247252

248253
var wrapper = new JobOptionsWrapper { Options = jobOptions };
@@ -343,6 +348,8 @@ public async Task<ActionResult<MultiRunJobDto>> UpdateMultiRunJob(
343348

344349
var jobOptions = _mapper.Map<MultiRunJobOptions>(dto);
345350

351+
ValidateGuestLocalFileUsage(HttpContext.GetApiUser(), jobOptions);
352+
346353
var jsonSettings = new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.Auto };
347354

348355
var wrapper = new JobOptionsWrapper { Options = jobOptions };
@@ -1071,4 +1078,28 @@ private static JobType GetJobType(Job job) =>
10711078
ProxyCheckJob => JobType.ProxyCheck,
10721079
_ => throw new NotImplementedException()
10731080
};
1081+
1082+
private static void ValidateGuestLocalFileUsage(ApiUser apiUser, MultiRunJobOptions jobOptions)
1083+
{
1084+
if (apiUser.Role is not UserRole.Guest)
1085+
{
1086+
return;
1087+
}
1088+
1089+
if (jobOptions.DataPool is FileDataPoolOptions fileDataPool
1090+
&& !fileDataPool.FileName.IsSubPathOf(Globals.UserDataFolder))
1091+
{
1092+
throw new ForbiddenException(
1093+
ErrorCode.FileOutsideAllowedPath,
1094+
$"Guest users cannot access files outside of the {Globals.UserDataFolder} folder");
1095+
}
1096+
1097+
if (jobOptions.ProxySources.OfType<FileProxySourceOptions>().Any(ps =>
1098+
ScriptExtensions.Contains(Path.GetExtension(ps.FileName), StringComparer.OrdinalIgnoreCase)))
1099+
{
1100+
throw new ForbiddenException(
1101+
ErrorCode.ScriptFileNotAllowed,
1102+
"Guest users cannot use script-based proxy sources");
1103+
}
1104+
}
10741105
}

‎OpenBullet2.Web/Controllers/WordlistController.cs‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,14 @@ namespace OpenBullet2.Web.Controllers;
2222
[TypeFilter<GuestFilter>]
2323
[ApiVersion("1.0")]
2424
public class WordlistController(IWordlistRepository wordlistRepo,
25-
IConfiguration config,
2625
IGuestRepository guestRepo, IObjectMapper mapper,
2726
ILogger<WordlistController> logger) : ApiController
2827
{
29-
private readonly string _baseDir = config.GetSection("Settings")
30-
.GetValue<string>("UserDataFolder") ?? "UserData";
3128
private readonly IGuestRepository _guestRepo = guestRepo;
3229
private readonly ILogger<WordlistController> _logger = logger;
3330
private readonly IObjectMapper _mapper = mapper;
3431
private readonly IWordlistRepository _wordlistRepo = wordlistRepo;
32+
private static readonly string[] ScriptExtensions = [".bat", ".ps1", ".sh"];
3533

3634
/// <summary>
3735
/// Get a wordlist by id.
@@ -114,14 +112,14 @@ public async Task<ActionResult<WordlistDto>> Create(CreateWordlistDto dto,
114112

115113
// If the user is a guest, make sure they are not accessing
116114
// anything outside the UserData folder.
117-
if (apiUser.Role is UserRole.Guest && !dto.FilePath.IsSubPathOf(_baseDir))
115+
if (apiUser.Role is UserRole.Guest && !dto.FilePath.IsSubPathOf(Globals.UserDataFolder))
118116
{
119117
_logger.LogWarning(
120118
"Guest user {Username} tried to access a file outside of the allowed directory while creating a wordlist at {FilePath}",
121119
apiUser.Username, dto.FilePath);
122120

123121
throw new ForbiddenException(ErrorCode.FileOutsideAllowedPath,
124-
$"Guest users cannot access files outside of the {_baseDir} folder");
122+
$"Guest users cannot access files outside of the {Globals.UserDataFolder} folder");
125123
}
126124

127125
// Make sure the file exists
@@ -171,8 +169,28 @@ public async Task<ActionResult<WordlistDto>> Create(CreateWordlistDto dto,
171169
[RequestSizeLimit(long.MaxValue)]
172170
public async Task<ActionResult<WordlistFileDto>> Upload(IFormFile file, CancellationToken cancellationToken)
173171
{
172+
var uploadedFileName = Path.GetFileName(file.FileName);
173+
174+
if (string.IsNullOrWhiteSpace(uploadedFileName))
175+
{
176+
throw new BadRequestException(
177+
ErrorCode.ValidationError, "The uploaded file name is invalid");
178+
}
179+
180+
if (ScriptExtensions.Contains(Path.GetExtension(uploadedFileName),
181+
StringComparer.OrdinalIgnoreCase))
182+
{
183+
throw new ForbiddenException(
184+
ErrorCode.ScriptFileNotAllowed,
185+
"Uploading script files as wordlists is not allowed");
186+
}
187+
188+
var safeFileName = FileUtils.ReplaceInvalidFileNameChars(uploadedFileName);
189+
var wordlistsDir = Path.Combine(Globals.UserDataFolder, "Wordlists");
190+
Directory.CreateDirectory(wordlistsDir);
191+
174192
var path = FileUtils.GetFirstAvailableFileName(
175-
Path.Combine(_baseDir, "Wordlists", file.FileName));
193+
Path.Combine(wordlistsDir, safeFileName));
176194

177195
await using var fileStream = System.IO.File.OpenWrite(path);
178196
await file.CopyToAsync(fileStream, cancellationToken);

‎OpenBullet2.Web/Exceptions/ApiException.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ public static class ErrorCode
104104
/// </summary>
105105
public const string FileOutsideAllowedPath = "FILE_OUTSIDE_ALLOWED_PATH";
106106

107+
/// <summary>
108+
/// Script files are not allowed for this operation.
109+
/// </summary>
110+
public const string ScriptFileNotAllowed = "SCRIPT_FILE_NOT_ALLOWED";
111+
107112
/// <summary>
108113
/// Remote resource not found.
109114
/// </summary>

‎RuriLib/Models/Proxies/ProxySource/FileProxySource.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ public async override Task<IEnumerable<Proxy>> GetAllAsync(CancellationToken can
3939
var fileExtension = (Path.GetExtension(FileName) ?? string.Empty).ToLowerInvariant();
4040
if (fileExtension.Length != 0 && supportedScripts.Contains(fileExtension))
4141
{
42+
if (UserId > 0)
43+
{
44+
throw new UnauthorizedAccessException(
45+
"Script-based proxy sources are not allowed for guest users");
46+
}
47+
4248
var locker = asyncLocker ?? throw new ObjectDisposedException(nameof(FileProxySource));
4349

4450
// The file is a script.

0 commit comments

Comments
 (0)