899237 - report_index is relying on raw crash's `InstallTime` being a safe integer to cast

Status: RESOLVED FIXED

(Socorro :: Webapp, task)

Description

[Peter Bengtsson [:peterbe]]

This raw crash has an example of a weird `InstallTime`
https://crash-stats.mozilla.com/api/RawCrash/?crash_id=6958e08a-2d61-4cc8-bcaa-340c52130729&format=meta

That's why this fails:
https://errormill.mozilla.org/webtools/socorro-crashstats-prod/group/55413/

Comment 1

[Robert Helmer [:rhelmer]]

No trusting the client evar :)

Comment 2

[Robert Helmer [:rhelmer]]

Pretty sure this is my fault, without looking at blame I remember working on this :(

I wonder if we could push input validation somewhere common, maybe the model? RawCrash is literally just the HTTP POST params other than the binary dump file, so it could contain anything at all.

I don't *think* the way we're handling it could cause any security issues, but there is a subset of required fields that make a crash invalid if they don't exist and appear in an expected format, InstallTime being one of them.

Comment 3

[Peter Bengtsson [:peterbe]]

The business logic is that it attempts to transform `InstallTime` to `install_time` IF the field *exists*. We should simply extend that to if the field exists and *is valid*.

Comment 4

[Robert Helmer [:rhelmer]]

(In reply to Peter Bengtsson [:peterbe] from comment #3)
> The business logic is that it attempts to transform `InstallTime` to
> `install_time` IF the field *exists*. We should simply extend that to if the
> field exists and *is valid*.

Sounds good. The same should go for any of the subset of required fields in RawCrash which is why I suggested centralizing it.

Comment 5

[Peter Bengtsson [:peterbe]]

That was easy https://github.com/mozilla/socorro/pull/1370

Comment 6

[[github robot]]

Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/425865d6f02350c962e4da81809a68fe55dd34b6
fixes bug 899237 - report_index is relying on raw crash's `InstallTime` as int, r=rhelmer

Comment 7

[Lonnen :lonnen]

Changes for this bug are going out in 56.