Generator-jhipster: Security issues in the user management

[laxaa]

Overview of the issue

On the user management webpage: http://localhost:8080/admin/user-management, it is possible to sort columns and the following URL can be observed when sorting by Login:
http://localhost:8080/admin/user-management?page=1&sort=login,desc
The data displayed on the screen is from the generators/server/templates/src/main/java/package/domain/User.java.ejs which contains data from the jhi_user database table. The issue is that some properties of the model contain security related data and that an administrator can sort them out. For example:
http://localhost:8080/admin/user-management?page=1&sort=password,desc
In this case, the password property is the password_hash column of the database.

Motivation for or Use Case

Even if this feature is supposed to be only for administrative purpose, thus requiring a high privilege account, the password and resetKey properties should not be exposed.
Indeed, the resetKey of any user could be retrieved with the following methodology:

• create a user on the application
• reset the password and retrieve the key, then sort the users and compare the resetKey with a user we want to retrieve its resetKey
• re-reset the password so the first character change again and compare it again. By dichotomy, this technique might help recover each character of the resetKey of a certain user

This technique would require a lot of requests to generate resetKey that would allow retrieving the resetKey of another user because of its 20 character lengths. It is pretty unlikely that a working attack could be made.

This technique can not be applied to the password as the bcrypt hashing algorithm is used and contains a random salt that is unkown to an attacker.

Moreover, this technique could be applied to any other data exposed by the model.

Reproduce the error

Explains on last paragraph

Related issues

N/A

Suggest a Fix

Security related elements specific to a user should not be present in a model used for arbitrary sorting to a user/administrator.

JHipster Version(s)

[email protected] /home/laxa/Documents/blog
└── [email protected]

##### **JHipster configuration**

.yo-rc.json file

{
  "generator-jhipster": {
    "promptValues": {
      "packageName": "org.jhipster.blog"
    },
    "jhipsterVersion": "6.10.1",
    "applicationType": "monolith",
    "baseName": "blog",
    "packageName": "org.jhipster.blog",
    "packageFolder": "org/jhipster/blog",
    "serverPort": "8080",
    "authenticationType": "jwt",
    "cacheProvider": "ehcache",
    "enableHibernateCache": true,
    "websocket": false,
    "databaseType": "sql",
    "devDatabaseType": "mysql",
    "prodDatabaseType": "mysql",
    "searchEngine": false,
    "messageBroker": false,
    "serviceDiscoveryType": false,
    "buildTool": "maven",
    "enableSwaggerCodegen": false,
    "jwtSecretKey": "YourJWTSecretKeyWasReplacedByThisMeaninglessTextByTheJHipsterInfoCommandForObviousSecurityReasons",
    "embeddableLaunchScript": false,
    "useSass": true,
    "clientPackageManager": "npm",
    "clientFramework": "react",
    "clientTheme": "none",
    "clientThemeVariant": "",
    "creationTimestamp": 1598011041769,
    "testFrameworks": [],
    "jhiPrefix": "jhi",
    "entitySuffix": "",
    "dtoSuffix": "DTO",
    "otherModules": [],
    "enableTranslation": false,
    "blueprints": []
  }
}

Entity configuration(s) entityName.json files generated in the .jhipster directory

N/A

Browsers and Operating System

Not applicable here

• [ X] Checking this box is mandatory (this is just to show you read everything)

[pascalgrimaud]

Thanks for opening this issue @laxaa

cc @jhipster/developers : we need to take a look to this important ticket and confirm this issue
cc @avdev4j @nassimerrahoui @adilabed

[atomfrede]

I must admit I don't understand the attack vector here completely (but I trust you here @laxaa), but I totally agree we should not allow sorting by password(hash) or reset keys. I can reproduce it by the way (sorting by password).

[cbornet]

+1 to remove the sort. If I understand well, by looking at the position of your own account compared to the others, you can get reduced intervals of possible reset keys which would lower the barrier for a brute force attack. It would still be hard to get a reset keys that way but as it's easier than a brute force over the full key space, a fix is required.

[avdev4j]

I'm taking this one

[pascalgrimaud]

Thanks @avdev4j
Can you do 2 PR, for master and maintenance branch plz ?

[laxaa]

+1 to remove the sort. If I understand well, by looking at the position of your own account compared to the others, you can get reduced intervals of possible reset keys which would lower the barrier for a brute force attack. It would still be hard to get a reset keys that way but as it's easier than a brute force over the full key space, a fix is required.

Hello everyone, thank you for the reply. Indeed, my explication is not the best and @cbornet explained exactly what I meant :). This attack vector is really improbable considering the amount of requests that would be needed.

[cbornet]

Thanks for spotting this vulnerability @laxaa . This is clearly one that only someone with very good security background can spot !