CVE-2020-17495 is being flagged in Pyup Safety scans of this package through v1.2.1

[MartinFalatic]

I'm opening this to raise the visibility of CVE-2020-17495, as seen in the current released package version v1.2.1 as mentioned in #142, which is now getting flagged in Pyup Safety scans.

https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-17495

https://nvd.nist.gov/vuln/detail/CVE-2020-17495

╞══════════════════════════════════════════════════════════════════════════════╡
│ REPORT                                                                       │
│ checked 178 packages, using pyup.io's DB                                     │
╞════════════════════════════╤═══════════╤══════════════════════════╤══════════╡
│ package                    │ installed │ affected                 │ ID       │
╞════════════════════════════╧═══════════╧══════════════════════════╧══════════╡
│ django-celery-results      │ 1.2.1     │ <=1.2.1                  │ 38678    │
╞══════════════════════════════════════════════════════════════════════════════╡
│ Django-celery-results through 1.2.1 stores task results in the database.     │
│ Among the data it stores are the variables passed into the tasks. The        │
│ variables may contain sensitive cleartext information that does not belong   │
│ unencrypted in the database. See CVE-2020-17495.                             │
╘══════════════════════════════════════════════════════════════════════════════╛

[robvdl]

I'm wondering why it actually needs to store the input arguments at all and if not storing the task arguments would satisfy the CVE fully so that it can be resolved. I see it can also store a traceback when you can just use something else like Sentry for that.

[kazauwa]

@auvipy, are there any plans on fixing this? Do you guys need any help?

[lieryan]

IMO, this CVE is just completely bogus and incorrect, and the entry should be removed from the vulnerability database. Whether or not task arguments and task results are sensitive is up to the application to decide, not celery or the results collector.

If you're developing an application that is handling sensitive data and you need to pass in sensitive arguments and/or return a sensitive results, you should be encrypting those when creating the task and/or when returning the result. It's also your responsibility to implement any necessary access control if you only want certain users viewing the task results, Django's admin permissions contains enough tools for you to implement any access policy you needed. In any case, those would be an application vulnerability, not a library vulnerability..

It's not the library's job to decide whether or not your tasks contains sensitive information, especially not when the main selling point of this library and the reason you install this library in the first place is specifically so you can query those information so that privileged users can actually view task arguments/results. If you don't want to be able to view tasks argument/results in Django and if that does not fulfil your security requirements, you wouldn't be installing this library in the first place. If you just want a basic results backend that stores results in a database, you should've just used Celery's built-in database backend rather than django-celery-results.

The way this CVE was written, we'd also have to file a CVE for fopen()/fwrite() method in all OS, because file contents "may contain sensitive cleartext information that does not belong unencrypted in the file", which is just completely ridiculous.

[robvdl]

I'd hate to say it but although the CVE sounds bogus, I believe it isn't because we shouldn't storing input arguments automatically. Now most people won't put sensitive information in input arguments, but if they did it would automatically be stored in the db and can't be turned off.

My argument is "why do we even need to store input arguments at all???" I've never found this useful, and if the task itself wants to save something to the database, let it do this by itelf or let ot do some logging, but don't store input arguments automatically by default (I've never found this useful), that is what got you the CVE in the first place.

[lieryan]

After 7.5 million years of mulling over, we came to the conclusion that the Answer to the Ultimate Question is 42.

Storing input arguments are useful, we use it all the time to diagnose issues with a task, re-run it a failing task if needed, as well as for tracking down where tasks came from, among many other things. The use case for this library is for humans to view task results, what's the point of being able to view a tasks' results if you don't know the context of why the task ran in the first place?

Why is the input arguments considered sensitive, but not the task results? In any case, that doesn't even matter, it is the application's responsibility to decide what is or is not sensitive, not the library's.

I've never found this useful

If you don't find what the library does to be useful, why are you using this library in the first place. Celery already comes with a built-in database backend that supports every DBMS that this library supports, you should use that instead if you just want a basic task results backend.

[robvdl]

I suppose my argument to that is "that is what logging is for".

[KyeRussell]

Who issued the CVE, and through which CNA? Seeing as everyone and their dog can get issued a CVE these days, you'd think that the vendor would get a right of reply.

[MartinFalatic]

In my original post here I have a link to it from Mitre. The assigning CNA appears to be MITRE Corporation itself.

[robvdl]

A new version of django celery results (2.0.0) has been released, and though it does not yet address this CVE it no longer sets off the pyup safety scanner (for now).

[arnau126]

@robvdl, it's not the default, but if you don't want to store input arguments to the db, you can pass custom strings to argsrepr and kwargsrepr when sending task, which will be stored to the db instead of the actual input arguments.
(https://docs.celeryproject.org/en/stable/userguide/tasks.html#hiding-sensitive-information-in-arguments)

from celery import shared_task

@shared_task
def debug_task(x, y=0, s=True):
    pass

debug_task.apply_async(
    args=(3, ), kwargs={'y': 2, 's': True},
    argsrepr="(*, )", kwargsrepr="{'y': *, 's': *}",
)

or just empty strings:

debug_task.apply_async(
    args=(3, ), kwargs={'y': 2, 's': True},
    argsrepr="", kwargsrepr="",
)

I agree with @lieryan :

It's not the library's job to decide whether or not your tasks contains sensitive information

[robvdl]

Sorry that is totally missing the point. The point is that in time 2.0.0 will also get marked as having a CVE and that will still set off pyup the security scanner we use in the CI pipeline for example.

Having the manually pass things in argsrepr etc sounds really convoluted to me. I would have thought that if a breaking change (v2.0.0 a major version) needed to be made anyway then I would have personally just removed input arguments and recommend users should do their own logging in tasks as that is really what a logger is for.

It would have resolved the CVE as well. I still absolutely don't get the point of storing input arguments for a library that is all about a celery output. And believe that logging is for this purpose instead.

[arnau126]

Celery already has a setting to decided whether to write input arguments to the backend or not: result_extended (default False).
However, the custom backend that this package implements (DatabaseBackend) does not check this setting.

Checking this setting before storing input arguments to the db will fix the issue.

[robvdl]

That sounds reasonable to me. It would probably need to be documented too I would imagine.

[robvdl]

But it does say this setting defaults to False so it would affect every user. It seems an unusual named setting results_extended.

[auvipy]

I am looking for the right fix here