Skip to content

Commit 17be60e

Browse files
msykmsyk
committed
Replace rand with random_int and remove fallback try-catch.
Updated all instances where `rand` was used with the more secure `random_int` function. Removed unnecessary try-catch blocks for `random_int` as it no longer requires fallbacks. Also improved string comparison by replacing equality checks with `hash_equals` for safer, timing-attack resistant comparisons.
1 parent 2f4ac64 commit 17be60e

9 files changed

Lines changed: 25 additions & 60 deletions

File tree

‎dist-docs/sample_schema_all_apply.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ psql --quiet -f "${distDocDir}/sample_schema_pgsql.sql" -h localhost test_db
1212
rm /var/db/im/sample.sq3
1313
sqlite3 /var/db/im/sample.sq3 < "${distDocDir}/sample_schema_sqlite.sql"
1414

15-
#mysql -uroot < "${distDocDir}/../spec/run/additionals_mysql.sql"
16-
#psql --quiet -f "${distDocDir}/../spec/run/additionals_postgresql.sql" -h localhost test_db
17-
#sqlite3 /var/db/im/sample.sq3 < "${distDocDir}/../spec/run/additionals_sqlite.sql"
15+
mysql -uroot < "${distDocDir}/../spec/run/additionals_mysql.sql"
16+
psql --quiet -f "${distDocDir}/../spec/run/additionals_postgresql.sql" -h localhost test_db
17+
sqlite3 /var/db/im/sample.sq3 < "${distDocDir}/../spec/run/additionals_sqlite.sql"
1818

‎src/php/DB/Support/ActionHandlers/ActionHandler.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ protected function prepareCheckAuthentication(): bool
127127
$proxy->code2FA = $this->storedCredential ? substr($this->storedCredential, 48, $proxy->digitsOf2FACode) : "";
128128
case'email': // Send mail containing 2FA code.
129129
$proxy->code2FA = $this->storedCredential ? substr($this->storedCredential, 48, $proxy->digitsOf2FACode) : "";
130-
break;
130+
break;
131131
case 'authenticator':
132132
default:
133133
$proxy->code2FA = $proxy->PostData['code2FA'] ?? "";
@@ -169,7 +169,7 @@ protected function checkAuthenticationCommon(): bool
169169
$this->storedCredential, $proxy->clientId, $proxy->hashedPassword);
170170
Logger::getInstance()->setDebugMessage("[checkAuthenticationCommon] credential={$proxy->credential} "
171171
. "storedChallenge={$this->storedChallenge} clientId={$proxy->clientId} hashedPassword={$proxy->hashedPassword}", 2);
172-
if ($proxy->credential === $referingCredential) {
172+
if (hash_equals($proxy->credential, $referingCredential)) {
173173
if ($proxy->required2FA) {
174174
$userName = $proxy->dbSettings->getCurrentUser();
175175
[, , $email, , $secret] = $proxy->dbClass->authHandler->getLoginUserInfo($userName);
@@ -256,7 +256,7 @@ protected function sessionStorageCheckAuth(): bool
256256
Logger::getInstance()->setDebugMessage(
257257
"[sessionStorageCheckAuth] hashedPassword={$proxy->hashedPassword}/hmac_value={$hmacValue}", 2);
258258
if (strlen($proxy->hashedPassword) > 0) {
259-
if ($proxy->paramResponse === $hmacValue) {
259+
if (hash_equals($proxy->paramResponse, $hmacValue)) {
260260
Logger::getInstance()->setDebugMessage("[sessionStorageCheckAuth] sha1 hash used.", 2);
261261
if ($proxy->migrateSHA1to2) {
262262
$salt = hex2bin(substr($proxy->hashedPassword, -8));

‎src/php/DB/Support/Proxy_Auth.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ public function checkMediaToken(string $user, string $token): bool
316316
$uid = $this->dbClass->authHandler->authSupportGetUserIdFromUsername($user);
317317
if ($uid) {
318318
$storedChallenge = $this->authDbClass->authHandler->authSupportCheckMediaToken($uid);
319-
if (strlen($storedChallenge) === 48 && $storedChallenge === $token) { // ex.fc0d54312ce33c2fac19d758
319+
if (strlen($storedChallenge) === 48 && hash_equals($storedChallenge, $token)) { // ex.fc0d54312ce33c2fac19d758
320320
$returnValue = true;
321321
}
322322
}

‎src/php/GenerateJSCode.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,8 @@ public function generateInitialJSCode(?array $dataSource, ?array $options, ?arra
297297
if (is_null($remoteAddr) || $remoteAddr === FALSE) {
298298
$remoteAddr = '0.0.0.0';
299299
}
300-
$clientIdSeed = time() . $remoteAddr . mt_rand();
301-
$randomSecret = mt_rand();
300+
$clientIdSeed = time() . $remoteAddr . random_int();
301+
$randomSecret = random_int();
302302
$clientId = hash_hmac('sha256', $clientIdSeed, $randomSecret);
303303

304304
$this->generateAssignJS(

‎src/php/IMUtil.php‎

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -582,11 +582,7 @@ public static function randomDigit(int $digit): string
582582
{
583583
$resultStr = '';
584584
for ($i = 0; $i < $digit; $i++) {
585-
try {
586-
$code = random_int(0, 9);
587-
} catch (Exception $ex) {
588-
$code = rand(0, 10);
589-
}
585+
$code = random_int(0, 9);
590586
$resultStr .= $code;
591587
}
592588
return $resultStr;
@@ -600,11 +596,7 @@ public static function randomString(int $digit): string
600596
{
601597
$resultStr = '';
602598
for ($i = 0; $i < $digit; $i++) {
603-
try {
604-
$code = random_int(33, 126);
605-
} catch (Exception $ex) {
606-
$code = rand(33, 126);
607-
}
599+
$code = random_int(33, 126);
608600
$resultStr .= chr($code);
609601
}
610602
return $resultStr;
@@ -620,11 +612,7 @@ public static function challengeString(int $digit): string
620612
$len = strlen($chars);
621613
$resultStr = '';
622614
for ($i = 0; $i < $digit; $i++) {
623-
try {
624-
$code = random_int(0, $len - 1);
625-
} catch (Exception $ex) {
626-
$code = rand(0, $len - 1);
627-
}
615+
$code = random_int(0, $len - 1);
628616
$resultStr .= $chars[$code];
629617
}
630618
return $resultStr;
@@ -650,7 +638,7 @@ public static function generateChallenge(): string
650638
{
651639
$str = '';
652640
for ($i = 0; $i < 24; $i++) {
653-
$n = rand(1, 255);
641+
$n = random_int(1, 255);
654642
$str .= ($n < 16 ? '0' : '') . dechex($n);
655643
}
656644
return $str;
@@ -663,7 +651,7 @@ public static function generateSalt(): string
663651
{
664652
$str = '';
665653
for ($i = 0; $i < 4; $i++) {
666-
$n = rand(33, 126); // They should be an ASCII character for JS SHA1 lib.
654+
$n = random_int(33, 126); // They should be an ASCII character for JS SHA1 lib.
667655
$str .= chr($n);
668656
}
669657
return $str;
@@ -680,10 +668,10 @@ public static function generatePassword(int $digit): string
680668
$seedPunctuation = "#$%&";
681669
$str = '';
682670
for ($i = 0; $i < $digit - 1; $i++) {
683-
$n = rand(0, strlen($seed) - 1);
671+
$n = random_int(0, strlen($seed) - 1);
684672
$str .= substr($seed, $n, 1);
685673
}
686-
$n = rand(0, strlen($seedPunctuation) - 1);
674+
$n = random_int(0, strlen($seedPunctuation) - 1);
687675
$str .= substr($seedPunctuation, $n, 1);
688676
return $str;
689677
}
@@ -720,7 +708,7 @@ public static function generateCredentialWithRandomPW(int $digit, string $passwo
720708
{
721709
$password = '';
722710
for ($i = 0; $i < $digit; $i++) {
723-
$password .= chr(rand(32, 127));
711+
$password .= chr(random_int(32, 127));
724712
}
725713
return IMUtil::convertHashedPassword($password, $passwordHash, $alwaysGenSHA2);
726714
}
@@ -731,17 +719,9 @@ public static function generateCredentialWithRandomPW(int $digit, string $passwo
731719
public static function generateRandomPW(): string
732720
{
733721
$str = '';
734-
try {
735-
$limit = random_int(15, 20);
736-
} catch (Exception $ex) {
737-
$limit = rand(15, 20);
738-
}
722+
$limit = random_int(15, 20);
739723
for ($i = 0; $i < $limit; $i++) {
740-
try {
741-
$n = random_int(33, 126); // They should be an ASCII character for JS SHA1 lib.
742-
} catch (Exception $ex) {
743-
$n = rand(33, 126); // They should be an ASCII character for JS SHA1 lib.
744-
}
724+
$n = random_int(33, 126); // They should be an ASCII character for JS SHA1 lib.
745725
$str .= chr($n);
746726
}
747727
return $str;

‎src/php/Media/AWSS3.php‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,7 @@ public function processing(Proxy $db, ?string $url, ?array $options, array $fil
157157
$targetFieldName = $field[$counter];
158158
$dirPath = $contextName . DIRECTORY_SEPARATOR
159159
. $keyField . "=" . $keyValue . DIRECTORY_SEPARATOR . $targetFieldName;
160-
try {
161-
$rand4Digits = random_int(1000, 9999);
162-
} catch (Exception $ex) {
163-
$rand4Digits = rand(1000, 9999);
164-
}
160+
$rand4Digits = random_int(1000, 9999);
165161
$objectKey = $dirPath . '/'
166162
. (!is_null($this->customFileName)
167163
? ($this->customFileName . ($counter > 1 ? "_" . $counter : ""))

‎src/php/Media/Dropbox.php‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,7 @@ public function processing(Proxy $db, ?string $url, ?array $options, array $fil
133133
$targetFieldName = $field[$counter];
134134
$dirPath = $contextName . DIRECTORY_SEPARATOR
135135
. $keyField . "=" . $keyValue . DIRECTORY_SEPARATOR . $targetFieldName;
136-
try {
137-
$rand4Digits = random_int(1000, 9999);
138-
} catch (Exception $ex) {
139-
$rand4Digits = rand(1000, 9999);
140-
}
136+
$rand4Digits = random_int(1000, 9999);
141137

142138
$objectPath = $this->rootInDropbox . '/' . $dirPath . '/'
143139
. (!is_null($this->customFileName)

‎src/php/Media/FileSystem.php‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,7 @@ private function decideFilePath(Proxy $db, bool $noOutput, ?array $options, str
173173
. $this->justifyPathComponent($keyField, $uploadFilePathMode) . "="
174174
. $this->justifyPathComponent($keyValue, $uploadFilePathMode) . DIRECTORY_SEPARATOR
175175
. $this->justifyPathComponent($targetFieldName, $uploadFilePathMode);
176-
try {
177-
$rand4Digits = random_int(1000, 9999);
178-
} catch (Exception $ex) {
179-
$rand4Digits = rand(1000, 9999);
180-
}
176+
$rand4Digits = random_int(1000, 9999);
181177
$filePartialPath = $dirPath . '/'
182178
. (!is_null($this->customFileName)
183179
? ($this->customFileName . ($counter > 1 ? "_" . $counter : ""))

‎src/php/Media/FileURL.php‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
class FileURL extends UploadingSupport implements DownloadingSupport
3232
{
3333
private ?string $customFileName = null;
34+
3435
/** Handles file upload processing, including CSV import if specified.
3536
* @param Proxy $db The database proxy instance.
3637
* @param string|null $url The redirect URL on error.
@@ -44,8 +45,8 @@ class FileURL extends UploadingSupport implements DownloadingSupport
4445
* @param array|null $dataSource Data source definition.
4546
* @param array|null $dbSpec Database specification.
4647
* @param int $debug Debug level.
47-
* @throws Exception If an error occurs during processing.
4848
* @return void
49+
* @throws Exception If an error occurs during processing.
4950
*/
5051
public function processing(Proxy $db, ?string $url, ?array $options, array $files, bool $noOutput, array $field,
5152
string $contextName, ?string $keyField, ?string $keyValue,
@@ -171,11 +172,7 @@ private function decideFilePath(Proxy $db, bool $noOutput, ?array $options,
171172
. $this->justifyPathComponent($keyField, $uploadFilePathMode) . "="
172173
. $this->justifyPathComponent($keyValue, $uploadFilePathMode) . DIRECTORY_SEPARATOR
173174
. $this->justifyPathComponent($targetFieldName, $uploadFilePathMode);
174-
try {
175-
$rand4Digits = random_int(1000, 9999);
176-
} catch (Exception $ex) {
177-
$rand4Digits = rand(1000, 9999);
178-
}
175+
$rand4Digits = random_int(1000, 9999);
179176
$filePartialPath = $dirPath . '/'
180177
. (!is_null($this->customFileName)
181178
? ($this->customFileName . ($counter > 1 ? "_" . $counter : ""))

0 commit comments

Comments
 (0)