Skip to content

Commit 09bcfd3

Browse files
[ADVAPP-2293]: Fix security issue with unauthorised file copying from anywhere on S3 to a form submission (#2208)
* add s3 file upload validation * Update code formatting and copyright headers * add validations in controller --------- Co-authored-by: jayushi-canyon <170343024+jayushi-canyon@users.noreply.github.com>
1 parent 7ea9823 commit 09bcfd3

2 files changed

Lines changed: 52 additions & 14 deletions

File tree

app-modules/form/src/Actions/GenerateSubmissibleValidation.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,20 @@ public function fields(array $blocks, Collection $fields): array
8989
$field->getKey() => $rules->merge($blockRules)->all(),
9090
];
9191

92+
// Security: Add file upload path validation for upload fields
93+
if ($field->type === 'upload' || $field->type === 'educatable-upload') {
94+
$nestedRules['*.path'] = [
95+
'required_with:' . $field->getKey(),
96+
'string',
97+
'regex:/^tmp\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-zA-Z0-9]+$/i',
98+
];
99+
$nestedRules['*.originalFileName'] = [
100+
'required_with:' . $field->getKey(),
101+
'string',
102+
'max:255',
103+
];
104+
}
105+
92106
foreach ($nestedRules as $nestedKey => $nestedRule) {
93107
$result[$field->getKey() . '.' . $nestedKey] = $nestedRule;
94108
}

app-modules/form/src/Actions/ProcessSubmissionField.php

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
use App\Settings\ImportSettings;
5959
use Illuminate\Support\Facades\Storage;
6060
use Illuminate\Support\Str;
61+
use InvalidArgumentException;
6162
use libphonenumber\NumberParseException;
6263
use libphonenumber\PhoneNumberFormat;
6364
use libphonenumber\PhoneNumberUtil;
@@ -84,20 +85,28 @@ public function __invoke(Submission $submission, string $fieldId, mixed $respons
8485

8586
foreach ($response as $file) {
8687
$key = ltrim($file['path'], '/');
87-
88-
$media = $fieldSubmission
89-
->addMediaFromDisk($key, 's3')
90-
->usingFileName($file['originalFileName'] ?? basename($key))
91-
->toMediaCollection('files', 's3');
92-
93-
Storage::disk('s3')->delete($key);
94-
$fieldSubmission->update([
95-
'response' => [
96-
'media_id' => $media->id,
97-
'file_name' => $media->file_name,
98-
'original_name' => $file['originalFileName'] ?? $media->file_name,
99-
],
100-
]);
88+
$this->validatePath($key);
89+
90+
if (! Storage::exists($key)) {
91+
continue;
92+
}
93+
94+
try {
95+
$media = $fieldSubmission
96+
->addMediaFromDisk($key, 's3')
97+
->usingFileName($file['originalFileName'] ?? basename($key))
98+
->toMediaCollection('files', 's3');
99+
100+
$fieldSubmission->update([
101+
'response' => [
102+
'media_id' => $media->id,
103+
'file_name' => $media->file_name,
104+
'original_name' => $file['originalFileName'] ?? $media->file_name,
105+
],
106+
]);
107+
} finally {
108+
Storage::disk('s3')->delete($key);
109+
}
101110
}
102111
}
103112

@@ -126,6 +135,21 @@ public function __invoke(Submission $submission, string $fieldId, mixed $respons
126135
}
127136
}
128137

138+
protected function validatePath(string $path): void
139+
{
140+
if (str_contains($path, '..') || str_contains($path, '//')) {
141+
throw new InvalidArgumentException('Invalid path: path traversal not allowed');
142+
}
143+
144+
if (! str_starts_with($path, 'tmp/')) {
145+
throw new InvalidArgumentException('Invalid path: must be within tmp/ directory');
146+
}
147+
148+
if (! preg_match('/^tmp\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-zA-Z0-9]+$/i', $path)) {
149+
throw new InvalidArgumentException('Invalid path: does not match expected format');
150+
}
151+
}
152+
129153
protected function handleEducatableEmailField(Submission $submission, mixed $response): void
130154
{
131155
$submissible = $submission->submissible;

0 commit comments

Comments
 (0)