Skip to content

Commit 015cbcc

Browse files
NicoDucouNicoDucou
committed
Security: remove possibility for get_user_popup to be called recursively with all user_id by using a none predictable hash instead of the user_id -refs BT#21323
1 parent 329e208 commit 015cbcc

File tree

10 files changed

+107
-15
lines changed

10 files changed

+107
-15
lines changed

‎main/inc/ajax/user_manager.ajax.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@
121121

122122
$courseId = (int) $request->get('course_id');
123123
$sessionId = (int) $request->get('session_id');
124-
$userId = (int) $request->get('user_id');
124+
$hash = (string) $request->get('hash');
125+
$userId = (int) UserManager::decryptUserHash($hash);
125126

126127
$user_info = api_get_user_info($userId);
127128

‎main/inc/lib/CoursesAndSessionsCatalog.class.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1780,6 +1780,7 @@ public static function getFormattedSessionsBlock(array $sessions)
17801780
$plugin = \BuyCoursesPlugin::create();
17811781
$isThisSessionOnSale = $plugin->getBuyCoursePluginPrice($session);
17821782

1783+
$userIdHash = UserManager::generateUserHash($coachId);
17831784
$sessionsBlock = [
17841785
'id' => $session->getId(),
17851786
'name' => $session->getName(),
@@ -1788,7 +1789,7 @@ public static function getFormattedSessionsBlock(array $sessions)
17881789
'nbr_users' => $session->getNbrUsers(),
17891790
'coach_id' => $coachId,
17901791
'coach_url' => $generalCoach
1791-
? api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&user_id='.$coachId
1792+
? api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&hash='.$userIdHash
17921793
: '',
17931794
'coach_name' => $coachName,
17941795
'coach_avatar' => UserManager::getUserPicture($coachId, USER_IMAGE_SIZE_SMALL),

‎main/inc/lib/api.lib.php‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1820,7 +1820,8 @@ function _api_format_user($user, $add_password = false, $loadAvatars = true)
18201820
$result['profile_url'] = api_get_path(WEB_CODE_PATH).'social/profile.php?u='.$user_id;
18211821

18221822
// Send message link
1823-
$sendMessage = api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&user_id='.$user_id;
1823+
$userIdHash = UserManager::generateUserHash($user_id);
1824+
$sendMessage = api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&hash='.$userIdHash;
18241825
$result['complete_name_with_message_link'] = Display::url(
18251826
$result['complete_name_with_username'],
18261827
$sendMessage,
@@ -10615,9 +10616,23 @@ function api_decrypt_ldap_password(string $encryptedText): string
1061510616
} else {
1061610617
return false;
1061710618
}
10619+
10620+
return api_decrypt_hash($encryptedText,$secret);
10621+
}
10622+
10623+
/**
10624+
* Decrypt sent hash encoded with secret
10625+
*
10626+
* @param $encryptedText The hash text to be decrypted
10627+
* @param $secret The secret used to encoded the hash
10628+
*
10629+
* @return string The decrypted text or false
10630+
*/
10631+
function api_decrypt_hash(string $encryptedHash, string $secret): string
10632+
{
1061810633
$secret = hex2bin($secret);
10619-
$iv = base64_decode(substr($encryptedText, 0, 16), true);
10620-
$data = base64_decode(substr($encryptedText, 16), true);
10634+
$iv = base64_decode(substr($encryptedHash, 0, 16), true);
10635+
$data = base64_decode(substr($encryptedHash, 16), true);
1062110636
$tag = substr($data, strlen($data) - 16);
1062210637
$data = substr($data, 0, strlen($data) - 16);
1062310638

@@ -10634,3 +10649,31 @@ function api_decrypt_ldap_password(string $encryptedText): string
1063410649
return false;
1063510650
}
1063610651
}
10652+
10653+
/**
10654+
* Encrypt sent data with secret
10655+
*
10656+
* @param $data The text to be encrypted
10657+
* @param $secret The secret to use encode data
10658+
*
10659+
* @return string The encrypted text or false
10660+
*/
10661+
function api_encrypt_hash($data, $secret)
10662+
{
10663+
$secret = hex2bin($secret);
10664+
$iv = random_bytes(12);
10665+
$tag = '';
10666+
10667+
$encrypted = openssl_encrypt(
10668+
$data,
10669+
'aes-256-gcm',
10670+
$secret,
10671+
OPENSSL_RAW_DATA,
10672+
$iv,
10673+
$tag,
10674+
'',
10675+
16
10676+
);
10677+
10678+
return base64_encode($iv) . base64_encode($encrypted . $tag);
10679+
}

‎main/inc/lib/course.lib.php‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2222,7 +2222,8 @@ public static function getTeachersFromCourse($courseId, $loadAvatars = true)
22222222
$userPicture = UserManager::getUserPicture($teacher['user_id'], USER_IMAGE_SIZE_SMALL);
22232223
$teachers['avatar'] = $userPicture;
22242224
}
2225-
$teachers['url'] = $url.'&user_id='.$teacher['user_id'];
2225+
$userIdHash = UserManager::generateUserHash($teacher['user_id']);
2226+
$teachers['url'] = $url.'&hash='.$userIdHash;
22262227
$listTeachers[] = $teachers;
22272228
}
22282229

@@ -2255,7 +2256,8 @@ public static function getTeacherListFromCourseCodeToString(
22552256
$teacher['lastname']
22562257
);
22572258
if ($add_link_to_profile) {
2258-
$url = api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&user_id='.$teacher['user_id'];
2259+
$userIdHash = UserManager::generateUserHash($teacher['user_id']);
2260+
$url = api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&hash='.$userIdHash;
22592261
$teacher_name = Display::url(
22602262
$teacher_name,
22612263
$url,
@@ -2337,7 +2339,9 @@ public static function get_coachs_from_course($session_id = 0, $courseId = 0, $l
23372339
$row['avatar'] = $loadAvatars
23382340
? UserManager::getUserPicture($row['user_id'], USER_IMAGE_SIZE_SMALL)
23392341
: '';
2340-
$row['url'] = "$url&user_id={$row['user_id']}";
2342+
$userIdHash = UserManager::generateUserHash($row['user_id']);
2343+
$row['url'] = $url.'&hash='.$userIdHash;
2344+
$row['hash'] = $userIdHash;
23412345

23422346
$coaches[] = $row;
23432347
}
@@ -2368,7 +2372,8 @@ public static function get_coachs_from_course_to_string(
23682372
foreach ($coachList as $coach_course) {
23692373
$coach_name = $coach_course['full_name'];
23702374
if ($add_link_to_profile) {
2371-
$url = api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&user_id='.$coach_course['user_id'].'&course_id='.$courseId.'&session_id='.$session_id;
2375+
$userIdHash = UserManager::generateUserHash($coach_course['user_id']);
2376+
$url = api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&hash='.$userIdHash.'&course_id='.$courseId.'&session_id='.$session_id;
23722377
$coach_name = Display::url(
23732378
$coach_name,
23742379
$url,

‎main/inc/lib/social.lib.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1220,9 +1220,10 @@ public static function show_social_menu(
12201220
'new-message.png',
12211221
$sendMessageText
12221222
);
1223+
$userIdHash = UserManager::generateUserHash($user_id);
12231224
$sendMessageUrl = api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?'.http_build_query([
12241225
'a' => 'get_user_popup',
1225-
'user_id' => $user_id,
1226+
'hash' => $userIdHash,
12261227
]);
12271228

12281229
$links .= '<li>';

‎main/inc/lib/statistics.lib.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,9 +415,10 @@ public static function getActivitiesData(
415415
}
416416

417417
// User id.
418+
$userIdHash = UserManager::generateUserHash($row[6]);
418419
$row[5] = Display::url(
419420
$row[5],
420-
api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&user_id='.$row[6],
421+
api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?a=get_user_popup&hash='.$userIdHash,
421422
['class' => 'ajax']
422423
);
423424

‎main/inc/lib/usermanager.lib.php‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8201,4 +8201,42 @@ private static function getGravatar(
82018201

82028202
return $url;
82038203
}
8204+
8205+
/**
8206+
* return user hash based on user_id and loggedin user's salt
8207+
*
8208+
* @param int user_id id of the user for whom we need the hash
8209+
*
8210+
* @return string containing the hash
8211+
*/
8212+
public static function generateUserHash(int $user_id): string
8213+
{
8214+
$currentUserId = api_get_user_id();
8215+
$userManager = self::getManager();
8216+
/** @var User $user */
8217+
$user = self::getRepository()->find($currentUserId);
8218+
if (empty($user)) {
8219+
return false;
8220+
}
8221+
return rawurlencode(api_encrypt_hash($user_id, $user->getSalt()));
8222+
}
8223+
8224+
/**
8225+
* return decrypted hash or false
8226+
*
8227+
* @param string hash hash that is to be decrypted
8228+
*
8229+
* @return string
8230+
*/
8231+
public static function decryptUserHash(string $hash): string
8232+
{
8233+
$currentUserId = api_get_user_id();
8234+
$userManager = self::getManager();
8235+
/** @var User $user */
8236+
$user = self::getRepository()->find($currentUserId);
8237+
if (empty($user)) {
8238+
return false;
8239+
}
8240+
return api_decrypt_hash(rawurldecode($hash), $user->getSalt());
8241+
}
82048242
}

‎main/inc/lib/userportal.lib.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2033,8 +2033,9 @@ public function returnCoursesAndSessions(
20332033
$params['category_id'] = $session_box['category_id'];
20342034
$params['title'] = $session_box['title'];
20352035
$params['id_coach'] = $coachId;
2036+
$userIdHash = UserManager::generateUserHash($coachId);
20362037
$params['coach_url'] = api_get_path(WEB_AJAX_PATH).
2037-
'user_manager.ajax.php?a=get_user_popup&user_id='.$coachId;
2038+
'user_manager.ajax.php?a=get_user_popup&hash='.$userIdHash;
20382039
$params['coach_name'] = !empty($session_box['coach']) ? $session_box['coach'] : null;
20392040
$params['coach_avatar'] = UserManager::getUserPicture(
20402041
$coachId,

‎main/social/search.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,10 @@
9898
<em class="fa fa-user"></em> '.get_lang('SendInvitation').'</a>';
9999
}
100100

101+
$userIdHash = UserManager::generateUserHash($user_info['user_id']);
101102
$sendMessageUrl = api_get_path(WEB_AJAX_PATH).'user_manager.ajax.php?'.http_build_query([
102103
'a' => 'get_user_popup',
103-
'user_id' => $user_info['user_id'],
104+
'hash' => $userIdHash,
104105
]);
105106

106107
$sendMessage = Display::toolbarButton(

‎main/template/default/user_portal/classic_session.tpl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@
119119
<img src="{{ 'teacher.png'|icon(16) }}" width="16" height="16">
120120
{% for coach in item.coaches %}
121121
{{ loop.index > 1 ? ' | ' }}
122-
<a href="{{ _p.web_ajax ~ 'user_manager.ajax.php?' ~ {'a': 'get_user_popup', 'user_id': coach.user_id, 'session_id': row.id, 'course_id': item.real_id }|url_encode() }}"
122+
<a href="{{ _p.web_ajax ~ 'user_manager.ajax.php?' ~ {'a': 'get_user_popup', 'hash': coach.hash, 'session_id': row.id, 'course_id': item.real_id }|url_encode() }}"
123123
data-title="{{ coach.full_name }}" class="ajax">
124124
{{ coach.firstname }}, {{ coach.lastname }}
125125
</a>
@@ -186,4 +186,4 @@
186186
<!-- end view simple info -->
187187
{% endif %}
188188
</div>
189-
{% endfor %}
189+
{% endfor %}

0 commit comments

Comments
 (0)