Weak OwnerGroup check in ReqProxy

Open
#8,736 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
65/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Quiet
Tech stack
python
Domain
security

Research direction

Start in src/DIRAC/RequestManagementSystem/Service/ReqProxyHandler.py around lines 162-167 and trace how a submitted Request dict is validated and then stored for execution. Reproduce a request with no OwnerGroup, then add coverage showing that the missing group cannot bypass the OwnerGroup check and that the original request data is handled safely.

Written by the indexing model from the issue text.

Description

Hi,

When a user sends a Request dict to ReqProxy they include the requested credentials in the task. This is verified to ensure they are an allowed value for the submitter's account:

https://github.com/DIRACGrid/DIRAC/blob/6a71b0df14498984d5623062e40b54468f1eb688/src/DIRAC/RequestManagementSystem/Service/ReqProxyHandler.py#L162-L167

However if the OwnerGroup is missing from the submitted request, the check passes with any Owner (overwriting the Owner/Group on the Request object). The Request object is then thrown away and the original JSON is stored for execution whatever Owner value was originally included in the requestDict (the OwnerGroup is left as None in the request).

As the permission checks on the called function are keyed by the group properties I don't think this is a security problem, but it would still be good to make it more robust just in-case anything changes in the future.

Regards,
Simon

Dominant language
Python
Stars
126
Forks
191
Avg merge
1d 13h
Merged PRs (30d)
33

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from DIRACGrid/DIRAC

All issues in DIRACGrid/DIRAC

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.