Server can be flooded with empty crash reports

[asesh]

If this is the URL for reporting crash reports: http://192.168.56.101:8000/service/crash_report/. Now if you send an empty POST request, via Insomnia or some other tools then it will be automatically logged in the server and the crash report will be empty. We should do some validation check before accepting crash reports. @sandsmark @rkudiyarov @thekondr @yurtaev @shashkin

Do you guys have any idea on how to fix this issue ASAP? I am working on a fix in our test server but however only empty crash reports are being logged but in our production server, crash reports are being uploaded. Maybe it's because my test server doesn't support HTTPS, not sure though.

Thanks

[kyakovenko]

Hello @asesh,

Thank you for letting us know. We'll check this issue but I should say that we don't see a big threat in the case when empty crashes are uploaded.

[asesh]

@kyakovenko but our server can be flooded with empty reports by just sending a POST request. Let's say a bot sends 50k empty POST requests, you know what will happen. So it's better to check if a request contains valid data else we should simply reject it. I have managed to do some validation in our fork but it's still not the best solution out there:

omaha_server/crash/forms.py
@@ -52,6 +52,10 @@ def clean_archive(self):

  def clean_upload_file_minidump(self):
      file = self.cleaned_data["upload_file_minidump"]
     # Make sure crash minidump exists
     if file is None:
        raise forms.ValidationError('A valid minidump file is required')
     if file and file.name.endswith('.tar'):
          try:
              t_file = BytesIO(file.read())

omaha_server/crash/views.py
@@ -40,6 +40,11 @@ def dispatch(self, *args, **kwargs):

  def form_valid(self, form):
      meta = self.request.POST.dict()
     # Make sure meta information of crash report exists
     if bool(meta) is False:
         return HttpResponseBadRequest('Bad request for submitting crash reports')

      meta.pop("appid", None)
      meta.pop("userid", None)
      obj = form.save(commit=False)

Those changes will check if a crash dump and meta files exist else a request will be denied. It can still be fooled by empty crash dump file and meta file

[shashkin]

Hello @asesh,
I believe that any kind of protection can be fooled with dummy requests here, the question is: what is the probability of your server to be flooded with such requests and how far you would go to make this probability smaller. I think some basic protection (like checking the file exists and not empty) would be helpful, but it is always the client's decision to add any more protection layers in their forks.

[asesh]

@shashkin @kyakovenko Yes, it's the client's decision to add more protection but shouldn't we add more validation checks before accepting crash reports? I think we should. Like I said it's not going to be bullet proof but at least it will make our server a little bit safer.