Skip to content

Commit 5147240

Browse files
daveearleydaveearley
authored
SSRF / IDOR fixes (#982)
1 parent 040584c commit 5147240

7 files changed

Lines changed: 435 additions & 6 deletions

File tree

‎backend/app/Exceptions/Handler.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
66
use Sentry\Laravel\Facade as Sentry;
7-
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
7+
use Symfony\Component\Routing\Exception\ResourceNotFoundException as SymfonyResourceNotFoundException;
88
use Throwable;
99

1010
class Handler extends ExceptionHandler
@@ -15,7 +15,8 @@ class Handler extends ExceptionHandler
1515
* @var array
1616
*/
1717
protected $dontReport = [
18-
// Add exceptions that shouldn't be reported
18+
ResourceNotFoundException::class,
19+
SymfonyResourceNotFoundException::class,
1920
];
2021

2122
/**
@@ -54,7 +55,7 @@ public function report(Throwable $e)
5455
*/
5556
public function render($request, Throwable $exception)
5657
{
57-
if ($exception instanceof ResourceNotFoundException) {
58+
if ($exception instanceof ResourceNotFoundException || $exception instanceof SymfonyResourceNotFoundException) {
5859
return response()->json([
5960
'message' => $exception->getMessage() ?: 'Resource not found',
6061
], 404);

‎backend/app/Http/Actions/PromoCodes/GetPromoCodeAction.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
namespace HiEvents\Http\Actions\PromoCodes;
44

55
use HiEvents\DomainObjects\EventDomainObject;
6+
use HiEvents\DomainObjects\Generated\PromoCodeDomainObjectAbstract;
7+
use HiEvents\Exceptions\ResourceNotFoundException;
68
use HiEvents\Http\Actions\BaseAction;
79
use HiEvents\Repository\Interfaces\PromoCodeRepositoryInterface;
810
use HiEvents\Resources\PromoCode\PromoCodeResource;
@@ -18,12 +20,22 @@ public function __construct(PromoCodeRepositoryInterface $promoCodeRepository)
1820
$this->promoCodeRepository = $promoCodeRepository;
1921
}
2022

23+
/**
24+
* @throws ResourceNotFoundException
25+
*/
2126
public function __invoke(Request $request, int $eventId, int $promoCodeId): JsonResponse
2227
{
2328
$this->isActionAuthorized($eventId, EventDomainObject::class);
2429

25-
$codes = $this->promoCodeRepository->findById($promoCodeId);
30+
$promoCode = $this->promoCodeRepository->findFirstWhere([
31+
PromoCodeDomainObjectAbstract::ID => $promoCodeId,
32+
PromoCodeDomainObjectAbstract::EVENT_ID => $eventId,
33+
]);
2634

27-
return $this->resourceResponse(PromoCodeResource::class, $codes);
35+
if ($promoCode === null) {
36+
throw new ResourceNotFoundException(__('Promo code not found'));
37+
}
38+
39+
return $this->resourceResponse(PromoCodeResource::class, $promoCode);
2840
}
2941
}

‎backend/app/Http/Request/Webhook/UpsertWebhookRequest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,15 @@
55
use HiEvents\DomainObjects\Status\WebhookStatus;
66
use HiEvents\Http\Request\BaseRequest;
77
use HiEvents\Services\Infrastructure\DomainEvents\Enums\DomainEventType;
8+
use HiEvents\Validators\Rules\NoInternalUrlRule;
89
use Illuminate\Validation\Rule;
910

1011
class UpsertWebhookRequest extends BaseRequest
1112
{
1213
public function rules(): array
1314
{
1415
return [
15-
'url' => 'required|url',
16+
'url' => ['required', 'url', new NoInternalUrlRule()],
1617
'event_types.*' => ['required', Rule::in(DomainEventType::valuesArray())],
1718
'status' => ['nullable', Rule::in(WebhookStatus::valuesArray())],
1819
];

‎backend/app/Services/Application/Handlers/PromoCode/UpdatePromoCodeHandler.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use HiEvents\DomainObjects\Generated\PromoCodeDomainObjectAbstract;
77
use HiEvents\DomainObjects\PromoCodeDomainObject;
88
use HiEvents\Exceptions\ResourceConflictException;
9+
use HiEvents\Exceptions\ResourceNotFoundException;
910
use HiEvents\Helper\DateHelper;
1011
use HiEvents\Repository\Interfaces\EventRepositoryInterface;
1112
use HiEvents\Repository\Interfaces\PromoCodeRepositoryInterface;
@@ -26,9 +27,19 @@ public function __construct(
2627
/**
2728
* @throws ResourceConflictException
2829
* @throws UnrecognizedProductIdException
30+
* @throws ResourceNotFoundException
2931
*/
3032
public function handle(int $promoCodeId, UpsertPromoCodeDTO $promoCodeDTO): PromoCodeDomainObject
3133
{
34+
$promoCode = $this->promoCodeRepository->findFirstWhere([
35+
PromoCodeDomainObjectAbstract::ID => $promoCodeId,
36+
PromoCodeDomainObjectAbstract::EVENT_ID => $promoCodeDTO->event_id,
37+
]);
38+
39+
if ($promoCode === null) {
40+
throw new ResourceNotFoundException(__('Promo code not found'));
41+
}
42+
3243
$this->eventProductValidationService->validateProductIds(
3344
productIds: $promoCodeDTO->applicable_product_ids,
3445
eventId: $promoCodeDTO->event_id

‎backend/app/Validators/Rules/NoInternalUrlRule.php‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
<?php
2+
3+
namespace HiEvents\Validators\Rules;
4+
5+
use Closure;
6+
use Illuminate\Contracts\Validation\ValidationRule;
7+
8+
class NoInternalUrlRule implements ValidationRule
9+
{
10+
private const ALLOWED_SCHEMES = ['http', 'https'];
11+
12+
private const BLOCKED_HOSTS = [
13+
'localhost',
14+
'127.0.0.1',
15+
'::1',
16+
'0.0.0.0',
17+
];
18+
19+
private const BLOCKED_TLDS = [
20+
'.localhost',
21+
];
22+
23+
private const CLOUD_METADATA_HOSTS = [
24+
'169.254.169.254',
25+
'metadata.google.internal',
26+
'metadata.goog',
27+
];
28+
29+
public function validate(string $attribute, mixed $value, Closure $fail): void
30+
{
31+
if (!is_string($value)) {
32+
$fail(__('The :attribute must be a valid URL.'));
33+
return;
34+
}
35+
36+
$parsedUrl = parse_url($value);
37+
if ($parsedUrl === false || !isset($parsedUrl['host'])) {
38+
$fail(__('The :attribute must be a valid URL.'));
39+
return;
40+
}
41+
42+
$scheme = strtolower($parsedUrl['scheme'] ?? '');
43+
if (!in_array($scheme, self::ALLOWED_SCHEMES, true)) {
44+
$fail(__('The :attribute must use http or https protocol.'));
45+
return;
46+
}
47+
48+
$host = strtolower($parsedUrl['host']);
49+
50+
// Handle IPv6 addresses wrapped in brackets
51+
if (str_starts_with($host, '[') && str_ends_with($host, ']')) {
52+
$host = substr($host, 1, -1);
53+
}
54+
55+
if ($this->isBlockedHost($host)) {
56+
$fail(__('The :attribute cannot point to localhost or internal addresses.'));
57+
return;
58+
}
59+
60+
if ($this->isBlockedTld($host)) {
61+
$fail(__('The :attribute cannot use reserved domain names.'));
62+
return;
63+
}
64+
65+
if ($this->isCloudMetadataHost($host)) {
66+
$fail(__('The :attribute cannot point to cloud metadata endpoints.'));
67+
return;
68+
}
69+
70+
if ($this->isPrivateIpAddress($host)) {
71+
$fail(__('The :attribute cannot point to private or internal IP addresses.'));
72+
return;
73+
}
74+
}
75+
76+
private function isBlockedHost(string $host): bool
77+
{
78+
return in_array($host, self::BLOCKED_HOSTS, true);
79+
}
80+
81+
private function isBlockedTld(string $host): bool
82+
{
83+
foreach (self::BLOCKED_TLDS as $tld) {
84+
if (str_ends_with($host, $tld)) {
85+
return true;
86+
}
87+
}
88+
return false;
89+
}
90+
91+
private function isCloudMetadataHost(string $host): bool
92+
{
93+
foreach (self::CLOUD_METADATA_HOSTS as $metadataHost) {
94+
if ($host === $metadataHost || str_ends_with($host, '.' . $metadataHost)) {
95+
return true;
96+
}
97+
}
98+
return false;
99+
}
100+
101+
private function isPrivateIpAddress(string $host): bool
102+
{
103+
$ip = gethostbyname($host);
104+
105+
if ($ip === $host && !filter_var($host, FILTER_VALIDATE_IP)) {
106+
return false;
107+
}
108+
109+
if (!filter_var($ip, FILTER_VALIDATE_IP)) {
110+
return false;
111+
}
112+
113+
if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) === false) {
114+
return true;
115+
}
116+
117+
if (str_starts_with($ip, '169.254.')) {
118+
return true;
119+
}
120+
121+
return false;
122+
}
123+
}

‎backend/tests/Unit/Services/Application/Handlers/PromoCode/UpdatePromoCodeHandlerTest.php‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
<?php
2+
3+
namespace Tests\Unit\Services\Application\Handlers\PromoCode;
4+
5+
use HiEvents\DomainObjects\Enums\PromoCodeDiscountTypeEnum;
6+
use HiEvents\DomainObjects\EventDomainObject;
7+
use HiEvents\DomainObjects\PromoCodeDomainObject;
8+
use HiEvents\Exceptions\ResourceNotFoundException;
9+
use HiEvents\Repository\Interfaces\EventRepositoryInterface;
10+
use HiEvents\Repository\Interfaces\PromoCodeRepositoryInterface;
11+
use HiEvents\Services\Application\Handlers\PromoCode\DTO\UpsertPromoCodeDTO;
12+
use HiEvents\Services\Application\Handlers\PromoCode\UpdatePromoCodeHandler;
13+
use HiEvents\Services\Domain\Product\EventProductValidationService;
14+
use Mockery as m;
15+
use Tests\TestCase;
16+
17+
class UpdatePromoCodeHandlerTest extends TestCase
18+
{
19+
private PromoCodeRepositoryInterface $promoCodeRepository;
20+
private EventProductValidationService $eventProductValidationService;
21+
private EventRepositoryInterface $eventRepository;
22+
private UpdatePromoCodeHandler $handler;
23+
24+
protected function setUp(): void
25+
{
26+
parent::setUp();
27+
28+
$this->promoCodeRepository = m::mock(PromoCodeRepositoryInterface::class);
29+
$this->eventProductValidationService = m::mock(EventProductValidationService::class);
30+
$this->eventRepository = m::mock(EventRepositoryInterface::class);
31+
32+
$this->handler = new UpdatePromoCodeHandler(
33+
$this->promoCodeRepository,
34+
$this->eventProductValidationService,
35+
$this->eventRepository
36+
);
37+
}
38+
39+
public function testHandleThrowsExceptionWhenPromoCodeNotFoundForEvent(): void
40+
{
41+
$promoCodeId = 1;
42+
$eventId = 2;
43+
$dto = new UpsertPromoCodeDTO(
44+
code: 'testcode',
45+
event_id: $eventId,
46+
applicable_product_ids: [],
47+
discount_type: PromoCodeDiscountTypeEnum::PERCENTAGE,
48+
discount: 10.0,
49+
expiry_date: null,
50+
max_allowed_usages: null
51+
);
52+
53+
$this->promoCodeRepository
54+
->shouldReceive('findFirstWhere')
55+
->once()
56+
->with([
57+
'id' => $promoCodeId,
58+
'event_id' => $eventId,
59+
])
60+
->andReturn(null);
61+
62+
$this->expectException(ResourceNotFoundException::class);
63+
$this->expectExceptionMessage('Promo code not found');
64+
65+
$this->handler->handle($promoCodeId, $dto);
66+
}
67+
68+
public function testHandleVerifiesPromoCodeBelongsToEvent(): void
69+
{
70+
$promoCodeId = 1;
71+
$eventIdFromRequest = 2;
72+
$attackerEventId = 999;
73+
74+
$dto = new UpsertPromoCodeDTO(
75+
code: 'testcode',
76+
event_id: $attackerEventId,
77+
applicable_product_ids: [],
78+
discount_type: PromoCodeDiscountTypeEnum::PERCENTAGE,
79+
discount: 10.0,
80+
expiry_date: null,
81+
max_allowed_usages: null
82+
);
83+
84+
$this->promoCodeRepository
85+
->shouldReceive('findFirstWhere')
86+
->once()
87+
->with([
88+
'id' => $promoCodeId,
89+
'event_id' => $attackerEventId,
90+
])
91+
->andReturn(null);
92+
93+
$this->promoCodeRepository
94+
->shouldNotReceive('updateFromArray');
95+
96+
$this->expectException(ResourceNotFoundException::class);
97+
98+
$this->handler->handle($promoCodeId, $dto);
99+
}
100+
101+
public function testHandleSuccessfullyUpdatesPromoCodeWhenOwnershipVerified(): void
102+
{
103+
$promoCodeId = 1;
104+
$eventId = 2;
105+
$dto = new UpsertPromoCodeDTO(
106+
code: 'testcode',
107+
event_id: $eventId,
108+
applicable_product_ids: [],
109+
discount_type: PromoCodeDiscountTypeEnum::PERCENTAGE,
110+
discount: 10.0,
111+
expiry_date: null,
112+
max_allowed_usages: null
113+
);
114+
115+
$existingPromoCode = m::mock(PromoCodeDomainObject::class);
116+
$existingPromoCode->shouldReceive('getId')->andReturn($promoCodeId);
117+
118+
$event = m::mock(EventDomainObject::class);
119+
$event->shouldReceive('getTimezone')->andReturn('UTC');
120+
121+
$updatedPromoCode = m::mock(PromoCodeDomainObject::class);
122+
123+
$this->promoCodeRepository
124+
->shouldReceive('findFirstWhere')
125+
->once()
126+
->with([
127+
'id' => $promoCodeId,
128+
'event_id' => $eventId,
129+
])
130+
->andReturn($existingPromoCode);
131+
132+
$this->eventProductValidationService
133+
->shouldReceive('validateProductIds')
134+
->once();
135+
136+
$this->promoCodeRepository
137+
->shouldReceive('findFirstWhere')
138+
->once()
139+
->with([
140+
'event_id' => $eventId,
141+
'code' => 'testcode',
142+
])
143+
->andReturn($existingPromoCode);
144+
145+
$this->eventRepository
146+
->shouldReceive('findById')
147+
->once()
148+
->with($eventId)
149+
->andReturn($event);
150+
151+
$this->promoCodeRepository
152+
->shouldReceive('updateFromArray')
153+
->once()
154+
->andReturn($updatedPromoCode);
155+
156+
$result = $this->handler->handle($promoCodeId, $dto);
157+
158+
$this->assertSame($updatedPromoCode, $result);
159+
}
160+
161+
protected function tearDown(): void
162+
{
163+
m::close();
164+
parent::tearDown();
165+
}
166+
}

0 commit comments

Comments
 (0)