3 ms·

So you'd go out and refactor a major security sensitive component (which dates to time before your career most likely) in a span of a single month for an emerge

by izacus 4y ago

So you'd go out and refactor a major security sensitive component (which dates to time before your career most likely) in a span of a single month for an emergency security patch deadline? That doesn't inspire a lot of confidence in your risk assesment and decision making. I'd do what Google did: rollout a patch that addresses the immediate danger and then backlog proper refactors over time.

gjsman-1000 4y ago
I don't think that is as much of an issue as the ridiculous process he had to go through. Think about that first security researcher. You literally found a Screen Unlock bypass (should be Priority #1, right?) - and Google just went and put fixing it on the backburner. If they will put something like that on the backburner, what else are they ignoring? It isn't confidence-inspiring. Edit: Also, knowing Google, what are the odds of your full refactor? "Temporary" fixes become permanent fixes quickly.
- izacus 4y ago
  > Edit: Also, knowing Google, what are the odds of your full refactor? "Temporary" fixes become permanent fixes quickly. Hahah, I wish that was only Google :D
- richardfey 4y ago
  Could have been sold for up to 300k or more on the black market.
  - pmontra 4y ago
    Maybe it was an already well known exploit. After all this was a duplicate and Google was sitting on it. Two people found it and reported it to Google. Why not a third one, and sold it?
- tehlike 4y ago
  Hahah it can go both ways. You can have 2 major rewrite over 3 years or you can have a new temporary-became-permanent bug fix.
krajzeg 4y ago
Their fix included a similarly large refactor, they just used the "security screen type" as a newly introduced parameter instead of something unique to the screen instance. I do agree that in the real world, sometimes you have to settle for a less-than-ideal solution. I hope my post reads less like "those people are idiots", which was not my intent, but more like: this specific fix isn't ideal, and knowing this type of code is live in a device doesn't fill me with confidence, even if I can understand reasons for why it was done that way.
- btown 4y ago
  Right? This was absolutely the "right" level of refactor for a hotfix, as the full refactor would introduce much more state management that could itself introduce bugs. And especially if behind the scenes there was a detailed audit of what things can currently access the current security screen, it would be fine for now. But I sincerely hope that in the postmortem, there would be a larger meta-discussion around code review practices and how something like this "global dismiss" became part of the API surface to begin with, and a sincere prioritization of a larger review within the backlog. Though with everyone on edge at this time in big tech, I doubt that ends up happening :(
- bragr 4y ago
  >Their fix included a similarly large refactor Their change is hardly a big refactor. This includes all the new code, all the parameter changes everywhere the function is used, and two additional test cases. This is a tiny change. >12 changed files with 102 additions and 26 deletions. [1] https://github.com/aosp-mirror/platform_frameworks_base/commit/ecbed81c3a331f2f0458923cc7e744c85ece96da https://github.com/aosp-mirror/platform_frameworks_base/comm...

by izacus 4y ago

gjsman-1000 4y ago

I don't think that is as much of an issue as the ridiculous process he had to go through. Think about that first security researcher. You literally found a Screen Unlock bypass (should be Priority #1, right?) - and Google just went and put fixing it on the backburner. If they will put something like that on the backburner, what else are they ignoring? It isn't confidence-inspiring. Edit: Also, knowing Google, what are the odds of your full refactor? "Temporary" fixes become permanent fixes quickly.

izacus 4y ago
> Edit: Also, knowing Google, what are the odds of your full refactor? "Temporary" fixes become permanent fixes quickly. Hahah, I wish that was only Google :D
richardfey 4y ago
Could have been sold for up to 300k or more on the black market.
- pmontra 4y ago
  Maybe it was an already well known exploit. After all this was a duplicate and Google was sitting on it. Two people found it and reported it to Google. Why not a third one, and sold it?
tehlike 4y ago
Hahah it can go both ways. You can have 2 major rewrite over 3 years or you can have a new temporary-became-permanent bug fix.

krajzeg 4y ago

Their fix included a similarly large refactor, they just used the "security screen type" as a newly introduced parameter instead of something unique to the screen instance. I do agree that in the real world, sometimes you have to settle for a less-than-ideal solution. I hope my post reads less like "those people are idiots", which was not my intent, but more like: this specific fix isn't ideal, and knowing this type of code is live in a device doesn't fill me with confidence, even if I can understand reasons for why it was done that way.

btown 4y ago
Right? This was absolutely the "right" level of refactor for a hotfix, as the full refactor would introduce much more state management that could itself introduce bugs. And especially if behind the scenes there was a detailed audit of what things can currently access the current security screen, it would be fine for now. But I sincerely hope that in the postmortem, there would be a larger meta-discussion around code review practices and how something like this "global dismiss" became part of the API surface to begin with, and a sincere prioritization of a larger review within the backlog. Though with everyone on edge at this time in big tech, I doubt that ends up happening :(
bragr 4y ago
>Their fix included a similarly large refactor Their change is hardly a big refactor. This includes all the new code, all the parameter changes everywhere the function is used, and two additional test cases. This is a tiny change. >12 changed files with 102 additions and 26 deletions. [1] https://github.com/aosp-mirror/platform_frameworks_base/commit/ecbed81c3a331f2f0458923cc7e744c85ece96da https://github.com/aosp-mirror/platform_frameworks_base/comm...