Skip to content

Commit 2884ab8

Browse files
fix(upload): replace predictable uniqid() with cryptographically secure random_bytes()
it doesnt really matter for security, but its a bad practice
1 parent 56b11a0 commit 2884ab8

1 file changed

Lines changed: 6 additions & 3 deletions

File tree

vulnerabilities/upload/source/impossible.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
// Check Anti-CSRF token
55
checkToken( $_REQUEST[ 'user_token' ], $_SESSION[ 'session_token' ], 'index.php' );
66

7-
87
// File information
98
$uploaded_name = $_FILES[ 'uploaded' ][ 'name' ];
109
$uploaded_ext = substr( $uploaded_name, strrpos( $uploaded_name, '.' ) + 1);
@@ -15,9 +14,13 @@
1514
// Where are we going to be writing to?
1615
$target_path = DVWA_WEB_PAGE_TO_ROOT . 'hackable/uploads/';
1716
//$target_file = basename( $uploaded_name, '.' . $uploaded_ext ) . '-';
18-
$target_file = md5( uniqid() . $uploaded_name ) . '.' . $uploaded_ext;
17+
18+
// Generate a single random name
19+
$random_name = bin2hex( random_bytes(16) ) . '.' . $uploaded_ext;
20+
21+
$target_file = $random_name;
1922
$temp_file = ( ( ini_get( 'upload_tmp_dir' ) == '' ) ? ( sys_get_temp_dir() ) : ( ini_get( 'upload_tmp_dir' ) ) );
20-
$temp_file .= DIRECTORY_SEPARATOR . md5( uniqid() . $uploaded_name ) . '.' . $uploaded_ext;
23+
$temp_file .= DIRECTORY_SEPARATOR . $random_name;
2124

2225
// Is it an image?
2326
if( ( strtolower( $uploaded_ext ) == 'jpg' || strtolower( $uploaded_ext ) == 'jpeg' || strtolower( $uploaded_ext ) == 'png' ) &&

0 commit comments

Comments
 (0)