PDF 전용 보안 파일 업로드
PDF 파일 만 허용 하는 PHP 7+를 사용하여 안전한 파일 업로드를 만들려고 합니다 . 다른 웹 사이트에서이 주제에 대한 많은 게시물을 찾았지만 이러한 방식으로 유해한 파일이 업로드되지 않도록 보장하는 완전한 솔루션을 찾지 못했습니다.
지금까지 다음 코드가 있습니다. 여기에 중요한 단계가 누락되었거나 코드에서 변경 또는 제거해야하는 것이 있는지 누군가가 말해 줄 수 있습니까?
( 참고 : 이전 x-pdf 파일 유형에는 관심이 없습니다.)
<?php
include 'session.php';
include 'header.php';
if (empty($_FILES['files'])) { echo json_encode(['error'=>'No files found for upload.']); return; } if(!empty($_POST['csrfToken'])) {
if(hash_equals($_SESSION['csrfToken'], $_POST['csrfToken'])) {
$postData = $_POST;
$files = $_FILES['files'];
$uploadRef = preg_replace('/[^A-Za-z0-9]/', '', $_GET['uploadRef']);
$categoryId = preg_replace('/[^A-Za-z0-9]/', '', $_GET['categoryId']);
$tags = preg_replace('/[^A-Za-z0-9,]/', '', $_GET['tagsList']);
$success = null; $paths= [];
$filenames = $files['name'];
for($i=0; $i < count($filenames); $i++){
if($_FILES['file']['error'] !== UPLOAD_ERR_OK) { die('Upload failed with error ' . $_FILES['file']['error']);
}
$fileTitle = $files['name'][$i]; $fileTitle = substr($fileTitle, 0 , (strrpos($fileTitle, ".")));
$fileExtensions = explode('.', basename($filenames[$i])); $fileExtension = strtolower(array_pop($fileExtensions)); $ok = false;
switch($fileExtension) { case 'pdf': $ok = true;
default:
die('Unknown/not permitted file type');
}
$finfo = finfo_open(FILEINFO_MIME_TYPE); $mime = finfo_file($finfo, $_FILES['file']['tmp_name']);
$ok = false; switch($mime) {
case 'application/pdf':
$ok = true; default: die('Unknown/not permitted file type'); } $uploadId = md5(uniqid()) . '_' . $i; $target = 'uploads' . DIRECTORY_SEPARATOR . $uploadId . '.' . $fileExtension;
if(move_uploaded_file($files['tmp_name'][$i], $target)) { $success = true;
$paths[] = $target;
$conn = new mysqli($dbHost, $dbUser, $dbPw, $dbName); if($conn->connect_error) {
exit($trans['errorConnectionFailedTxt'][$lang]);
}
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
$conn->set_charset('utf8mb4'); $stmt = $conn->prepare("INSERT INTO uploads (uploadId, uploadRef, categoryId, tags, fileTitle, fileExtension) VALUES (?, ?, ?, ?, ?, ?)"); $stmt->bind_param("ssssss", $uploadId, $uploadRef, $categoryId, $tags, $fileTitle, $fileExtension);
$stmt->execute(); $stmt->close();
$conn->close(); } else { $success = false;
break;
}
}
if ($success === true) { $output = [];
} elseif ($success === false) { $output = ['error'=>'Error while uploading images. Contact the system administrator'];
foreach ($paths as $file) {
unlink($file); } } else { $output = ['error'=>'No files were processed.'];
}
unset($postData); echo json_encode($output);
} else {
echo json_encode('invalid CSRF token');
}
} else {
echo json_encode('no CSRF token');
}
?>
답변
가드 진술
if(!empty($_POST['csrfToken'])) { if(hash_equals($_SESSION['csrfToken'], $_POST['csrfToken'])) {
토큰의 유효성을 검사하는 별도의 방법으로 수행해야한다고 생각합니다. 또한 이것이 HTTP 오류로 끝나야한다고 생각합니다 (다른 답변에서 알 수 있듯이).
위의 내용을 "guard statement"라고하며 메서드가 실행되기 전에 상태 또는 매개 변수가 유효한지 확인합니다. 유효하지 않은 경우 종료해야합니다. 종료는 로컬에서 수행 할 수 있습니다. 다음을 찾기 위해 메서드를 스크롤 할 필요는 없습니다.
} else {
echo json_encode('invalid CSRF token');
}
} else {
echo json_encode('no CSRF token');
}
끝에. 또한 단순히 메서드를 종료하면 들여 쓰기 레이어가 제거되어 메서드를 읽기가 덜 복잡해집니다.
불명확 한 정규 표현식
$uploadRef = preg_replace('/[^A-Za-z0-9]/', '', $_GET['uploadRef']);
이것은 또한 정규식이 이해하기 어렵 기 때문에 ( "무엇") 그렇게 많이는 아니지만 그것이 수행되는 이유 는 명확하지 않은 방법이어야합니다 .
성공?
$success = null;
성공은 부울이며 세 개의 값이있는 변수로 사용해서는 안됩니다. 두 개의 변수 또는 열거 형을 사용하십시오. 또한, $success끔찍한 나쁜 이름, 시도 filesUploaded또는 유사합니다.
그것이 count($filenames)0인지 미리 확인한다면 출력을 미리 설정하고 나머지 실행을 건너 뛸 수 있습니다 (guard 문을 기억하십시오). 프로그래밍은 복잡성을 제한하는 것입니다.
C 프로그래머 복구
$ok = false;
die어쨌든 필요하지 않습니까?
그렇게 독특하지 않음
uniqid()
"이 함수는 반환 값의 고유성을 보장하지 않습니다." 1 음, 맞습니다. 이것은 당신이 무엇을하든 끔찍하게 실패하기를 기다리고 있습니다. md5()그것을 사용 하더라도 아무것도 성취 할 수 없습니다 . 카운터가있는 확장은 병렬 프로세스에서 동일한 폴더를 사용하지 않는 한 도움이됩니다 (가능합니까?).
속편
SQL 문은 별도의 메서드 (예 :)에 있어야합니다 createFileUploadReport. 별도의 클래스를 만들면 다른보고 방법 (예 : 콘솔 또는 로그 파일에보고)을 대신 사용할 수도 있으므로 SQL 서버없이 메서드를 테스트 할 수 있습니다.
버그 / 로직 오류
두 switch문에는 하나의 케이스와 기본 케이스가 있습니다. 두 switch 문에서 defaultcase는 항상 실행됩니다. switch문 이 어떻게 작동 하는지 더 잘 이해하는 것이 유용 할 것입니다. 또한 한 가지 경우에 switch진술 을 사용하는 것은 이치에 맞지 않습니다 if. 간단한 진술로 충분합니다.
일관되지 않은 반환 유형
어떤 경우에는 배열이 전달되어 명령문 json_encode()과 함께 사용 echo되지만 다른 경우 die()에는 호출됩니다. json으로 인코딩 된 배열은이 스크립트가 비동기 로딩 메커니즘 (예 : AJAX)과 함께 사용된다고 믿게 만들지 만 배열과 간단한 문자열을 모두 찾지 않는 한 die()또는 exit()사용되는 경우 버려 질 가능성이 있습니다 .
그런 다음 업로드가 성공하면 분명히 다음 코드가 있습니다.
if ($success === true) { $output = [];
프런트 엔드 코드에는 그다지 도움이되지 않는 것 같습니다.
HTTP 응답 코드 를 사용하는 것도 현명 할 것입니다. 예를 들어 성공을 나타내려면 200, 유효하지 않은 입력을 나타내는 422 등입니다.
일찍 돌아옴
첫 번째 조건, 즉 if (empty($_FILES['files'])) {평가 true되면 JSON 응답이 적절한 오류 메시지와 함께 반환됩니다. 오류 메시지로 이어지는 다른 조건은 동일한 방식을 따라야합니다 (예 :의 빈 값 $_POST['csrfToken']등). 이렇게하면 나머지 코드의 들여 쓰기 양이 줄어 듭니다. 이 주제에 대한 자세한 내용은 Rafael Dohms가 키워드를 피하는 것처럼 코드를 간결하게 유지하는 여러 방법에 대해 이야기 하는 코드 정리에 대한이 프레젠테이션을 참조하십시오else ( 여기 슬라이드 참조 ).
루프 반복기 변수에 의존하지 않기 때문에이 블록은 for루프 위로 이동할 수 있습니다 .
if($_FILES['file']['error'] !== UPLOAD_ERR_OK) { die('Upload failed with error ' . $_FILES['file']['error']); }
MIME 유형 확인
finfo_open(FILEINFO_MIME_TYPE)MIME 유형을 가져 오기 위해 호출하는 대신 mime-content-type()함수를 사용할 수 있습니다.
$mime = mime-content-type($_FILES['file']['tmp_name'])
또한, MIME 타입이 브라우저에서 제공 될 수있다 $_FILES['files']['type'][$i]그것이 비록 " 당연히 그 값을 고려하지 않아도되므로 PHP 측에 체크하지. " 1
변수 $postData-정말 필요합니까?
postData할당 된 후에 는에 전달 된 한 지점에서만 사용되는 것으로 보입니다 unset(). 거의 필요한 것 같지 않습니다 ...
가변 필드 이름 검사
이 스크립트에 제출 된 양식은 포함되어 있지 않지만 필드에 대해서만 추측 할 수 있습니다. 스크립트는 다음과 $_FILES['files']같이 확인합니다 $_FILES['file']. 전자는 여러 파일을 업로드 할 가능성이 가장 높지만 실제로 두 개의 다른 파일 입력 필드가 있습니까?
루프의 데이터베이스 쿼리
이 코드는 현재 루프가 반복 될 때마다 데이터베이스에 레코드를 삽입합니다. 모든 레코드를 삽입하려면 하나의 문만 사용하는 것이 좋습니다. 이렇게하면 데이터베이스 연결이 최소화되고 (실행 시간이 늘어날 수 있음) 잘못된 데이터가 삽입되는 것을 방지 할 수 있습니다 (예 : 첫 번째 파일이 유효했지만 후속 파일이 유효하지 않은 경우).
문자를 대체하는 정규식
정규식 은 밑줄 문자를 포함하지만 문자 유형을 사용하여 단순화 할 수 있지만 제거해야합니까? 또한 다중 바이트 문자열을 안전하게 정리하지 못하므로 유니 코드 문자열을 지원해야하는 경우 고려하십시오.\w_preg_replace()