33

Last week I found a serious security vulnerability in our code during a PR review. Twice a month we have a sprint review where we all present our work for the past 2 weeks.

I'm excited that I protected the company, our clients and our developers from the damage that could have been caused. But I don't know how I should present it. The bug was a mistake anyone could have made, in no way does it imply that the devs that missed it are to blame for anything (besides initially I missed it as well). Bugs are a certainty in life and we all (including me) have created bugs.


So, how do I phrase it as to not downplay my success while at the same time not causing negative feelings to others? (the 2nd part is by far the most important)

Also, should I go into details on how exactly an attacker could use it and the damage it could have caused?

11
  • 5
    What are the agreements for logging and solving bugs in your process? (Since you mention sprint review I imagine some kind of scrum?) Did you just fix the bug while working on another assigned task? Did you not report it in a back log? Bottom line: are your colleagues even aware of it before the sprint review?
    – AsheraH
    Commented Mar 22, 2023 at 6:31
  • 3
    Was this bug somehow logged in your system of distributing work? If you are doing Scrum, maybe it was a user story? In other words, my first question to you would be, why were you working on this without the POs knowledge, why did you not do what was planned out to be done in the sprint planning instead?
    – nvoigt
    Commented Mar 22, 2023 at 6:58
  • 27
    Can you clarify that you were assigned to review someone's pull request and you found this potential problem and got them to fix it? That's very different than finding a vulnerability in released code using your own initiative. Finding problems during a code review is what you are supposed to do and not something I would seek praise for. Also, I'd be very unhappy if one of my code reviewers brought up the issues they found during a team meeting without a very good reason. (eg: this type of error is endemic and we should check all production code for it).
    – Eric Nolan
    Commented Mar 22, 2023 at 9:34
  • 7
    When this is found in the medical field, we call it a Near Miss/Close Call. I find the language pretty agreeable.
    – Turbo
    Commented Mar 22, 2023 at 20:58
  • 2
    Don't wait for the code review if it's important. Commented Mar 23, 2023 at 14:07

10 Answers 10

24

Since you're using terms like Sprint Review, I'm going to base this answer on the Scrum framework as it's defined in the Scrum Guide.

The Sprint Review is not the right place to bring this up.

The Sprint Review, as it's defined in the Scrum Guide, is an opportunity for the team and key stakeholders to collaborate. The most important outcome of this event is an improved understanding of the context and making adjustments to the Product Backlog (and perhaps the current Product Goal) to reflect the most valuable next steps for the team.

The Sprint Retrospective would be a more appropriate place to discuss detecting security vulnerabilities. On one hand, it's good that you were able to detect a security vulnerability before it was released, and talking about things that went well in the Sprint is part of the Sprint Retrospective. However, there's also ways to talk about how to improve the detection of security vulnerabilities. It could be worthwhile to talk about the processes and tools used to detect potential security vulnerabilities in the product and how security vulnerabilities could be detected even earlier, before a developer starts a code review. Or, even better, how to prevent design or coding errors that lead to security vulnerabilities in the first place. Although it's true that no person is to blame, the process and tools are letting potential vulnerabilities escape later into the development process than they need to.

Based on the discussion in the retrospective, there could be more opportunities for more technical discussions about how you found the defect so others can use those same techniques and the technical implications of such defects. These would be at the discretion of the team and could be part of retrospective action items to improve the team's ability to deliver high quality work.

8
  • 3
    Given that @snow already patched the bug (assuming I'm correctly understanding what was written in the question), I think this is the best answer. Commented Mar 22, 2023 at 15:28
  • 3
    It seems snow did nothing like that. He spotted a problem during a code review.
    – gnasher729
    Commented Mar 22, 2023 at 15:36
  • 3
    @gnasher729, aha, found the comment where he said that. So, he prevented the bug from being realized, which is even better. The sprint retrospective still seems like a good place to address the larger question: how do they reduce the likelihood of future security vulnerabilities being introduced? Commented Mar 22, 2023 at 18:23
  • 4
    @gnasher729 But, from the sounds of it, the bug almost wasn't prevented by review. That's a near-miss.
    – wizzwizz4
    Commented Mar 22, 2023 at 20:54
  • 3
    @gnasher729 I'm going to second what wizzwizz4 said. I work in an environment where security or safety bugs are extremely serious. Of course, this is why I advocate for the application of formal methods and not just "code review" to catch such things. Depending on the situation, this example of a near-miss might be a clarion call for stronger means of preventing getting so close to a potential vulnerability in the future. Commented Mar 23, 2023 at 16:40
79

Something like:

Hi Team, I noticed this bit of Code - something jumped out at me.

Based on my hunch, I decided to do a couple of tests - based off my Proof of Concept, there's a potential for exploit/exposure/privilege elevation/backdoor - I'd like to get this raised as a vulnerability and get it patched ASAP.

You're not pointing fingers - you spotted something, undertook some pro-active testing and want to raise it with the team.

Simple, to-the-point. You could also add (depending on if it's true or not) 'This looks similar to something I've seen before' which is the why you noticed it.

The reason I suggest going in with a PoC - is that it's a very neutral and objective way to show the problem: If we do X, then Y happens.

14
  • 23
    “Something jumped out at me. Based on a hunch” I’d be worried about anyone talking like this.
    – gnasher729
    Commented Mar 22, 2023 at 15:16
  • 14
    Yeah @gnasher this isn't excellently worded indeed, but it looks like a creative writing exercise, like more and more answers on this site do. "If I were writing a short novel, this is what my character would say."
    – CodeCaster
    Commented Mar 22, 2023 at 15:21
  • 38
    Nothing of this belongs in a sprint review. This should be talked about in the daily with the other devs. "I'd like to get this raised as a vulnerability and get it patched ASAP" is something to talk about with your PO or once again during the daily.
    – luk2302
    Commented Mar 22, 2023 at 16:18
  • 29
    @gnasher729 You could replace "hunch" with "sniff test", or with "it didn't feel right", or "heuristic" if you're trying for buzzword bingo, but essentially it amounts to the same thing. Previous experience suggested to you that it's likely to fail that way, and a small amount of testing proved this was the case. I'd be more worried about the rest of the team if they took exception to the word "hunch" compared to solid evidence of a bug. A good dev team will just say "thanks" and roll with it.
    – Graham
    Commented Mar 22, 2023 at 17:22
  • 9
    I mean, I use Hunch semi-regularly - and no one seems to mind or care. Commented Mar 22, 2023 at 18:26
57

If the vulnerability is still there, it’s a bug, and you report it as a bug. Show it to someone experienced, figure out how it can be exploited, prioritise it accordingly, and then it gets fixed.

If it is something that you found during a code review: Steady here. People make mistakes, that’s expected. That’s why we do code reviews. You spotted a mistake during a code review. That’s your job. Do you want praise for doing your job?

So here’s the thing: There was no “success”. You didn’t save the company lots of money. You did your job, reviewing code, finding problems, as you are supposed to. But then you tried to make sure that everyone knows and now you want to make sure more people know. I’d fear this could be a career limiting move.

8
  • 9
    yeah this is the correct answer for me. If OP were to bring it up then it would come across as them basically picking on another developer's coding skills (or lack of them) while blowing their own trumpet, which is not a good look
    – Aaron F
    Commented Mar 22, 2023 at 15:57
  • 2
    The problem in question seems to be "I have to present what I've done during these weeks, so how do I present that my CR was useful without implying any blame."
    – Džuris
    Commented Mar 22, 2023 at 20:05
  • 3
    @Java "it very well demonstrates what others will assume" - you're the one asking a question on a Q&A site, others are mere answerers who have to go by what you do and do not provide. Quotes from your question like "I protected the company, our clients and our developers" and "how do I [...] not downplay my success" do not quite speak humility.
    – CodeCaster
    Commented Mar 22, 2023 at 22:09
  • 4
    This answer seems to me to be accurate except for one detail: it is leaving out the possible collective learning. The OP said in a comment that everyone missed the vulnerability, so most likely on the team everyone has something important to learn. A competent team is built by sharing knowledge.
    – Tedpac
    Commented Mar 22, 2023 at 22:54
  • 3
    @Michael +1 I hate the attitude "that wasn't a success you were just doing your job", it's possible to be both!!
    – deep64blue
    Commented Mar 24, 2023 at 13:35
15

The bug was a mistake anyone could have made, in no way does it imply that the devs that missed it are to blame for anything (besides initially I missed it as well)

Personally, I think you are missing the point here. Finding the vulnerability is a good thing. The fact that you and everyone else missed it is really bad. This attitude frustrates me to no end. This idea that "shit happens" around preventable vulnerabilities is big part of why we have so many vulnerabilities.

Of course, asserting that there's some magic bullet that eliminates this kind of thing is ridiculous. It's going to require a lot of work to improve this situation. The question is: what are you and your team going to do about it? Where do you start?

That should be your focus in communicating the issue e.g.: "We all missed this, what can we do to prevent that from happening in the future?" Based on your description, you stumbled across this problem by accident. That is not a strategy. It's not a repeatable process. It is unlikely to be a reliable approach to prevent new vulnerabilities.

I get the sense that you are a junior to mid-level team member (please forgive me if I am wrong) and perhaps you feel it is above your level to address the software development life-cycle (SDLC). If so, you might want to discuss with senior technical and/or management about adding software security to your team's skill set.

1
  • 1
    I recommend adding bold to "you stumbled across this problem by accident. That is not a strategy. It's not a repeatable process." for emphasis. Those are the most important words on this page.
    – Ben Voigt
    Commented Mar 24, 2023 at 21:52
14

Developers find and fix bugs on a daily basis, with varying impact. Why do you think finding and fixing a security vulnerability deserves extra attention?

If you want recognition for your work: sure, bring up what you did during the appropriate meeting. Sprint review is not that, which is for demoing added value and determining next steps. Maybe remember it for your performance review?

If you want to explain to others that building software can cause data leaks, open up your servers/services to denial of service attacks or cause other vulnerabilities, then find the appropriate meeting to discuss that. Retrospective, probably, though that's more about the process, or some other meeting where you handle productivity, learning goals and whatnot.

3
  • 8
    A "serious security vulnerability" seems to me something that deserves special attention, especially in this case where it come from a single simple mistake that is easily overlooked: the others should know for them not to reproduce the mistake IMO
    – Kaddath
    Commented Mar 22, 2023 at 16:15
  • 3
    @Kaddath yes, in a daily, not in the sprint review with external stakeholders who are non technical and will most likely not care nor understand.
    – luk2302
    Commented Mar 22, 2023 at 16:20
  • 4
    Kaddath, to be honest, I wouldn’t take OPs word for it. And vulnerabilities don’t work like that. There’s no pattern.
    – gnasher729
    Commented Mar 22, 2023 at 18:00
11

Frame-Challenge

The best place to present this amazing success is not at the sprint review, but at your performance review. For the sprint review treat like any other bug that was found, and if there is something from a retrospective that should be discussed related to this do so.

As for the performance review, note this accomplishment down and how big of an impact it could have had if you had not caught it. Likewise do this for any other major accomplishments or amazing things you have done over the year. Then, when the performance review comes you can use all these things to explain why you deserve an excellent performance rating and hopefully get an appropriate raise.

4

To be honest, the answers by TheDemonLord and Thomas Owens should be combined. You want the tact and neutral language described by TheDemonLord, and bring this up in any other meeting, casual face-to-face chat, or scrum. Something like this should only be brought up during the sprint review if you have confirmed the vulnerability, done the research to determine this is a significant risk, and all efforts to handle this with the technical people have failed.

I'm not advocating for hiding security vulnerabilities, but the customer does not need to be burdened with every teeny-tiny detail of every problem you encounter. There are two important points here:

  • You found it before it went to production.
  • And the team gets it fixed before it goes to production.

Depending on the severity of the vulnerability, and how long it will be before this vulnerability hits production, bring this up during Scrum — in the very least. If time is of the essence, at least talk to the developer who introduced it (if you know this), and failing that talk to the lead developer. Do this as quickly as you can, assuming this is a serious security risk. If you are the lead developer, bring this up immediately with the team and get a volunteer to fix it. Failing that, pick someone and make them responsible.

Don't wait for some ceremony to happen. Address it like any other defect found during development and testing, except do this as quickly as possible.

After you decide the appropriate forum (which is not the sprint review), then consider phrasing the conversation like TheDemonLord did.

Please forgive the blatant plagiarism of their answer, but something akin to:

I noticed something that kind of jumped out at me in module/feature X. After a little research, it appears to be a potential exploit/exposure/privilege elevation/backdoor, and I would like to get this fixed ASAP.

Then work with the team to get the fix scheduled. Be sure to include your team lead. Consider including the project manager if a deadline or budget is at risk.

4

I'd take a slightly different approach here.

Last week I found a serious security vulnerability in our code during a PR review

Review for security should be, and usually is, part of code review. As long as it has not been integrated into a solution, it does not require any action from the part of the reviewer other than "potential/likely serious security issue (at X, because Y, link to solution or best practice), needs work". That's it.

Beyond that, it may well be worth sharing your finding in a sprint review or retrospective under the "lessons learned" aspect. If it really is something spectacular, systematic, other than a trivial honest mistake, then it should become part of the teams knowledge base.

4

I like to quip that there are no "bugs" in agile development. The formal concept of a Bug, which developers were afraid of, and everybody tried to hush-hush, and customers loved to blame the devs and let them fix it in their spare time is a relic of the old times of fixed-price, fixed-scope, fixed-time waterfall project management.

In the agile world, which you are presumably working in, based on your mention of 2-week sprints, bugs are the same as everything else; they begin life as a story (or whatever name you like to use) in the product backlog, eventually make their way into a sprint backlog, and finally get fixed and deployed. Even if you're not using SCRUM, where this particular terminology comes from, all agile process have these concepts in some form or other.

If someone starts talking about getting bugs fixed for free, this is a great time to point out that in exchange for treating bug-fixes like regular work, customers get the benefit of quick turnarounds for their implementations - we do not require them to write a 1000 page requirement book anymore and deliver in very quick succession.

So for you, this means that you simply write down everything there is to say on this bug in your task tracking system, whichever it may be; then you present it to the usual people (your PO, the team, etc.) in the relevant meetings that you regularly have (say, in a daily, or a refinement session). You can talk with your PO at any time (you do not need to wait for an official sprint event), of course, as can every other stakeholder.

Obviously, same as stories having different priorities, you can give a priority to this one. If you think it's really blatant (like a possibility of the VISA data of your customers being shown on a public web site...), then you would handle it differently than a minor code smell with a distant possibility of some attacker with deep knowledge of your code misusing it.

Note that even in the agile world, there is a separate concept called "incident". This is not a bug, and not a story, but something which inflicts problems on a production system and needs to be fixed right now (i.e., database down, "Error 500" on the login page, and so on). If your bug is of this calibre, then you would obviously do differently - follow your incident process instead of your story process.

2
  • The idea of a bug as distinct from a story isn't verboten in Agile. Our team has bugs and stories side-by-side. We treat them the same from an in-team perspective, but keeping the distinction is useful because outside of the team, bugs go through risk review and bugfix stories can result in patches, which we won't do for normal feature stories. That said, I wouldn't call what OP found a bug, but rather a review failure. The review and the fix should all be contained in the same feature story as the original development work; that is, if the "bug" isn't fixed, the original story isn't done. Commented Mar 24, 2023 at 19:45
  • Sure, one can have different types of stories, and different sub-processes per type of story - the point of my answer is that a bug is of the same general category as everything else a dev team would do, and that there is no reason whatsoever to hide it or treat it differently, implicitly (unless, well, you actually do have a good reason, as you are describing in your comment ;) ). @AndrewRay
    – AnoE
    Commented Mar 24, 2023 at 20:21
-2

So you found a security vulnerability and sat on it for a WEEK before even thinking out possibly disclosing it to someone?

THAT is the biggest problem, you should have followed proper channels within your team and indeed company INSTANTLY, at the very least inform the team lead or scrum master, the product owner, whomever is the lead on serious problems in your team. And if the code is already on an acceptance or production server, you should have INSTANTLY informed your company's security officer, and probably the CTO as well.

As is, you are seriously in arrears and negligent of your responsibilities as an employee and coworker, possibly criminally so (especially if the problem is already in production or UAT where it risks information being leaked outside the company).

I've found such bugs, and depending on the environment, did exactly that: inform the team leader, inform the CTO, inform the company security officer, and let them decide what to do and when. Some people may not like having a problem they are responsible for creating becoming known to others, most professionals would be happy that at least the problem has been identified and can now be solved, hopefully before it causes serious problems that can sink the company (and their careers). And yes, I've found problems serious enough they could have destroyed the company I was working for at the time were they to end up on a production server. Luckily I found them before that happened so all we had to deal with was telling customers the delivery of their next update would be delayed a while (rather than having to pay millions upon millions of Euros in fines for causing major data breaches).

1
  • 1
    You misunderstood the situation because you didn't read the comments where OP explained that it was immediately fixed and didn't reach production.
    – Ben Voigt
    Commented Mar 24, 2023 at 21:51

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .