Repository: CVEProject/cve-services
Pull request #567 (2022-03-09 12:15:04)

Issue #564
### Identify the Bug

Link to issue: #564 

### Description of the Change

Added a check to see if the org name used to authenticate is the same as the short name provided in the parameters. This means that if someone tries to enumerate users across CNAs they will get a 403 and the following error:

```json
{
    "error": "NOT_SAME_ORG_OR_SECRETARIAT",
    "message": "This information can only be viewed by the users of the same organization or the Secretariat."
}
```
The unit tests have been updated with this change.

### Alternate Designs

Doesn't really appear to any other designs as this pattern has been used on other /org/{shortname}/user endpoints.

### Possible Drawbacks

Can't really think of any as you wouldn't want user enumeration across CNAs.

### Verification Process

Replicated the PoC provided in the issue and proved that it is no longer valid. Also tested using a secretariat account and that works as expected.

Unit tests have been updated and are all 🟢 

### Release Notes

Fixed a bug in the /org/{shortname}/user/{username}/reset_secret API endpoint that could lead to user enumeration across CNAs.



Patch:
diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js
index b025cda0..9f7bb884 100644
--- a/src/controller/org.controller/org.controller.js
+++ b/src/controller/org.controller/org.controller.js
@@ -636,19 +636,24 @@ async function resetSecret (req, res, next) {
     const orgShortName = req.ctx.params.shortname
     const userRepo = req.ctx.repositories.getUserRepository()
     const orgRepo = req.ctx.repositories.getOrgRepository()
+    const isSecretariat = await orgRepo.isSecretariat(requesterShortName)
     const orgUUID = await orgRepo.getOrgUUID(orgShortName) // userUUID may be null if user does not exist
     if (!orgUUID) {
       logger.info({ uuid: req.ctx.uuid, messsage: orgShortName + ' organization does not exist.' })
       return res.status(404).json(error.orgDneParam(orgShortName))
     }
 
+    if (orgShortName !== requesterShortName && !isSecretariat) {
+      logger.info({ uuid: req.ctx.uuid, message: orgShortName + ' organization can only be viewed by the users of the same organization or the Secretariat.' })
+      return res.status(403).json(error.notSameOrgOrSecretariat())
+    }
+
     const oldUser = await userRepo.findOneByUserNameAndOrgUUID(username, orgUUID)
     if (!oldUser) {
       logger.info({ uuid: req.ctx.uuid, messsage: username + ' user does not exist.' })
       return res.status(404).json(error.userDne(username))
     }
 
-    const isSecretariat = await orgRepo.isSecretariat(requesterShortName)
     const isAdmin = await userRepo.isAdmin(requesterUsername, requesterShortName)
     // check if the user is not the requester or if the requester is not a secretariat
     if ((orgShortName !== requesterShortName || username !== requesterUsername) && !isSecretariat) {


Test patch:
diff --git a/test-http/src/test/org_user_tests/org_as_org_admin.py b/test-http/src/test/org_user_tests/org_as_org_admin.py
index 32279584..a40058fd 100644
--- a/test-http/src/test/org_user_tests/org_as_org_admin.py
+++ b/test-http/src/test/org_user_tests/org_as_org_admin.py
@@ -455,7 +455,7 @@ def test_org_admin_reset_diff_org_secret(org_admin_headers):
         headers=org_admin_headers
     )
     assert res.status_code == 403
-    response_contains_json(res, 'error', 'NOT_SAME_USER_OR_SECRETARIAT')
+    response_contains_json(res, 'error', 'NOT_SAME_ORG_OR_SECRETARIAT')
 
 
 def test_org_admin_reset_same_org_secret(org_admin_headers):
diff --git a/test/unit-tests/user/userResetSecretTest.js b/test/unit-tests/user/userResetSecretTest.js
index cc660b95..54883d93 100644
--- a/test/unit-tests/user/userResetSecretTest.js
+++ b/test/unit-tests/user/userResetSecretTest.js
@@ -273,7 +273,7 @@ describe('Testing the PUT /org/:shortname/user/:username/reset_secret endpoint i
 
           expect(res).to.have.status(403)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.notSameUserOrSecretariat()
+          const errObj = error.notSameOrgOrSecretariat()
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
@@ -302,7 +302,7 @@ describe('Testing the PUT /org/:shortname/user/:username/reset_secret endpoint i
 
           expect(res).to.have.status(403)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.notSameUserOrSecretariat()
+          const errObj = error.notSameOrgOrSecretariat()
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
@@ -331,7 +331,7 @@ describe('Testing the PUT /org/:shortname/user/:username/reset_secret endpoint i
 
           expect(res).to.have.status(403)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.notSameUserOrSecretariat()
+          const errObj = error.notSameOrgOrSecretariat()
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()