Fix/protect celery progress #2238

Closed

hansegucker wants to merge 0 commits from fix/protect-celery-progress into master

hansegucker commented

2021-05-15 12:11:34 +02:00

Owner

Close #379

Security advisory

Summary

We use Celery to run tasks asynchronously in the background. Sometimes there are scenarios that users start tasks in the frontend (e. g. a data import) and want to track the progress. For this case, we use celery-progress. celery-progress provides an unprotected API endpoint to allow the frontend JavaScript to refresh the displayed progress bar. Every user, even such who are not logged in, could call this API endpoint for every task.

How could the issue be exploited?

To get information from the API endpoint, an attacker would need a valid task ID. He could get one from an authorized user or by guessing. Iterating is not possible because the task IDs are UUID-like.

What information can be leaked?

In general, status information about tasks like SUCCESS, FAILURE or RETRY don't contain any personal information. But there are sometimes scenarios that custom progress/error messages are also exposed through this API. These messages could include personal information like "Importing the user Jane Doe failed ...".

How does the change fix the issue?

The changed code will link the corresponding user to each task whose progress should be tracked. By wrapping the default endpoint, we check if the user is logged in, and if the currently logged-in user is linked to the provided task ID. If the user isn't permitted to see the status, the endpoint will return a 404. This will also prevent fetching data for tasks that shouldn't be tracked through the frontend.

Is there general advice on how to prevent such issues in the future?

Every team member should check newly added views from third-party libraries if they have a sufficient, integrated authentication mechanism. Generally, adding new apps to the URLconf should be considered carefully. Maintainers should ask the developers in the review if they checked these points.

Close #379 ## Security advisory ### Summary We use Celery to run tasks asynchronously in the background. Sometimes there are scenarios that users start tasks in the frontend (e. g. a data import) and want to track the progress. For this case, we use celery-progress. celery-progress provides an **unprotected** API endpoint to allow the frontend JavaScript to refresh the displayed progress bar. **Every** user, even such who are not logged in, could call this API endpoint for **every** task. ### How could the issue be exploited? To get information from the API endpoint, an attacker would need a valid task ID. He could get one from an authorized user or by guessing. Iterating is not possible because the task IDs are UUID-like. ### What information can be leaked? In general, status information about tasks like SUCCESS, FAILURE or RETRY don't contain any personal information. But there are sometimes scenarios that custom progress/error messages are also exposed through this API. These messages could include personal information like "Importing the user Jane Doe failed ...". ### How does the change fix the issue? The changed code will link the corresponding user to each task whose progress should be tracked. By wrapping the default endpoint, we check if the user is logged in, and if the currently logged-in user is linked to the provided task ID. If the user isn't permitted to see the status, the endpoint will return a 404. This will also prevent fetching data for tasks that shouldn't be tracked through the frontend. ### Is there general advice on how to prevent such issues in the future? Every team member should check newly added views from third-party libraries if they have a sufficient, integrated authentication mechanism. Generally, adding new apps to the URLconf should be considered carefully. Maintainers should ask the developers in the review if they checked these points.

hansegucker commented

2021-05-15 12:12:43 +02:00

Author

Owner

changed the description

hansegucker closed this pull request

2021-05-15 12:26:54 +02:00