[vuln] OpenRedirect leads to XSS attack in login.php

[hi-unc1e]

Hi, guys, sorry for having chosen the wrong branch in here

Here i come again,

While I using testlink I had noticed that an arbitrary open redirect is not validating properly which may lead to credentials stolen of the user (self-xss).

• Risk: Medium
• Tested version: 1.9.20
• Environent: Windows10+ Chrome90.0.4430.212
• Credit：unc1e

Personally I prefer to split them into 2 vulns, Here's what i found

0x01 the vulnerable regex -> Unvalidated Redirects and Forward

Related code lies in login.php#371 , which is shown as follows

the regex-expression /linkto.php/ stands for the combination of [linkto]{1} + [\w]{1} + [php]{1}, in which . can be replaced by any signle character, so as a consequence.
These input could pass the validation, and reflected in the HTTP response when a user succeed in logining.

destination=ANYSITE/linkto1php

So the mediation may be like /^linkto\.php$/ , according to what your need.

What happened next may elevate this risk to a High-Level vuln.

0x02 Improper XSS validation

While deeply analysing the function of redirect(), i noticed an odd implementation.

$safeUrl = addslashes($url);

ONLY addslashes is used, which is SURELY NOT ENOUGH and VULNERABLE to XSS attack.
Which means if I craft a XSS payload in the URL, it could execute any Javascript Code in victim's browser, certainly after he's logged in.

Putting together

So the senario is:

1. Craft a URL, for example: http://testlink/login.php?viewer=123&destination=</script>linkto.php<script>alert(/xss/);//
2. Send it to a victim, the victim open that link
3. The victim logged with correct credentials
4. XSS attack is triggered, an attacker could steal the victim's valid cookie or craft a phishing page to the victim depends on different purpose.
5. Here is the PoC of this vuln ( You can see that Javascipt-script is executed on victim's browser. )
   

Mediation

For 0x01, you should validate the $destination properly according to Unvalidated_Redirects_and_Forwards_Cheat_Sheet

For 0x02, you should escape the user-input data ( htmlspecialchars() for instance ) properly, according to Cross_Site_Scripting_Prevention_Cheat_Sheet.html

[fmancardi]

Thanks for the details and your help I've implemented a different fix -> 83010fb would you mind to test it? -> -> you can use full code from branch testlink_1_9_20_fixed thanks

…

On Mon, May 31, 2021 at 4:20 AM hi-unc1e ***@***.***> wrote: Hi, guys, sorry for having chosen the wrong branch in here <#314> Here i come again, While I using testlink I had noticed that an arbitrary open redirect is not validating properly which may lead to credentials stolen of the user (self-xss). - Risk: High - Tested version: 1.9.20 - Environent: Windows10+ Chrome90.0.4430.212 - Credit： [image: image] <https://user-images.githubusercontent.com/67778054/119782947-552ecb00-beff-11eb-8b59-ba591c3d9bf3.png> Personally I prefer to split them into 2 vulns, Here's what i found 0x01 the vulnerable regex -> Unvalidated Redirects and Forward Related code lies in login.php#371 <https://github.com/TestLinkOpenSourceTRMS/testlink-code/blob/0e4b6a4ac3e5cb0c572fef86b7845b11259c687e/login.php#L371> , which is shown as follows [image: image] <https://user-images.githubusercontent.com/67778054/119778844-8f499e00-befa-11eb-9b48-efbc0718107d.png> the regex-expression /linkto.php/ stands for the combination of [linkto]{1} + [\w]{1} + [php]{1}, in which . can be replaced by any signle character, so as a consequence. These input could pass the validation, and reflected in the HTTP response when a user succeed in logining. destination=ANYSITE/linkto1php [image: image] <https://user-images.githubusercontent.com/67778054/119776575-b0f55600-bef7-11eb-83b1-382846cf4a39.png> So the mediation may be like /^linkto\.php$/ , according to what your need. What happened next may elevate this risk to a High-Level vuln. 0x02 Improper XSS validation While deeply analysing the function <https://github.com/TestLinkOpenSourceTRMS/testlink-code/blob/0e4b6a4ac3e5cb0c572fef86b7845b11259c687e/lib/functions/common.php#L528>of redirect(), i noticed an odd implementation. $safeUrl = addslashes($url); [image: image] <https://user-images.githubusercontent.com/67778054/119777231-73dd9380-bef8-11eb-8e95-49e948d12795.png> ONLY addslashes is used, which is SURELY NOT ENOUGH and VULNERABLE to XSS attack. Which means if I craft a XSS payload in the URL, it could execute any Javascript Code in victim's browser, certainly after he's logged in. [image: image] <https://user-images.githubusercontent.com/67778054/119779554-64137e80-befb-11eb-873a-8a6ba9f358c8.png> Putting together So the senario is: 1. Craft a URL, for example: http://testlink/login.php?viewer=123&destination= </script>linkto.php<script>alert(/xss/);// 2. Send it to a victim, the victim open that link 3. The victim logged with correct credentials 4. XSS attack is triggered, an attacker could steal the victim's valid cookie or craft a phishing page to the victim depends on different purpose. 5. Here is the PoC of this vuln ( You can see that Javascipt-script is executed on victim's browser. ) [image: image] <https://user-images.githubusercontent.com/67778054/119778078-9de38580-bef9-11eb-8122-921a812f1572.png> Mediation For 0x01, you should validate the $destination properly according to Unvalidated_Redirects_and_Forwards_Cheat_Sheet <https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html> For 0x02, you should escape the user-input data ( htmlspecialchars() for instance ) properly, according to Cross_Site_Scripting_Prevention_Cheat_Sheet.html <https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html> ------------------------------ You can view, comment on, or merge this pull request online at: #316 Commit Summary - typo error - fix (user contribution) - Fix #0008915 Update url in README.md - Added chosen search to reqTcAssign (#290) - fix: jira needs accountdId instead of user name while issue creation (#305) - fix unable to login with github (#299) File Changes - *M* README.md <https://github.com/TestLinkOpenSourceTRMS/testlink-code/pull/316/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5> (2) - *M* gui/templates/tl-classic/requirements/reqTcAssign.tpl <https://github.com/TestLinkOpenSourceTRMS/testlink-code/pull/316/files#diff-bedd166e9aa33b7fc1eb9b4805e5e37fc8b4ef052f86ff94e7fdfd674ca6160b> (15) - *M* install/sql/alter_tables/1.9.20/mysql/DB.1.9.20/step1/db_schema_update.sql <https://github.com/TestLinkOpenSourceTRMS/testlink-code/pull/316/files#diff-df8612759e33399f396d25a999f055731681f7f716d7892f2225f102d918ffaf> (33) - *M* lib/functions/oauth_providers/github.php <https://github.com/TestLinkOpenSourceTRMS/testlink-code/pull/316/files#diff-0cba6e685adf940a70369e8010a265d070244f33dafd2085bdf3bbc65b055b35> (1) - *M* lib/issuetrackerintegration/jirarestInterface.class.php <https://github.com/TestLinkOpenSourceTRMS/testlink-code/pull/316/files#diff-769c2cefdc15ea96ceedeb42a6b4f639cc03c236e4fdf10033468b2694181360> (10) - *M* third_party/fayp-jira-rest/Jira.php <https://github.com/TestLinkOpenSourceTRMS/testlink-code/pull/316/files#diff-9a830e8e1b35df7ebdf904a8622bad37307b7ded9fc5913f949cceacc4c4ae02> (38) Patch Links: - https://github.com/TestLinkOpenSourceTRMS/testlink-code/pull/316.patch - https://github.com/TestLinkOpenSourceTRMS/testlink-code/pull/316.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#316>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABZRMKTXMYGUMMR2HYAHDTTQLXARANCNFSM45Z7PFAQ> .

-- Francisco Mancardi

[hi-unc1e]

See my testing in 83010fb#r51662554