fixing-security-bugs.rst (9160B)
1 Fixing Security Bugs 2 ==================== 3 4 A bug has been reported as security-sensitive in Bugzilla and received a 5 security rating. 6 7 If this bug is private - which is most likely for a reported security 8 bug - **the process for patching is slightly different than the usual 9 process for fixing a bug**. 10 11 Here are security guidelines to follow if you’re involved in reviewing, 12 testing and landing a security patch. See 13 :ref:`Security Bug Approval Process` 14 for more details about how to request sec-approval and land the patch. 15 16 Keeping private information private 17 ----------------------------------- 18 19 A security-sensitive bug in Bugzilla means that all information about 20 the bug except its ID number are hidden. This includes the title, 21 comments, reporter, assignee and CC’d people. 22 23 A security-sensitive bug usually remains private until a fix is shipped 24 in a new release, **and after a certain amount of time to ensure that a 25 maximum number of users updated their version of Firefox**. Bugs are 26 usually made public after 6 months and a couple of releases. 27 28 From the moment a security bug has been privately reported to the moment 29 a fix is shipped and the bug is set public, all information about that 30 bug needs to be handled carefully in order to avoid an unmitigated 31 vulnerability becoming known and exploited before we release a 32 fix (0-day). 33 34 During a normal process, information about the nature of bug can be 35 accessed through: 36 37 - Bug comments (Bugzilla, GitHub issue) 38 - Commit message (visible on Bugzilla, tree check-ins and test servers) 39 - Code comments 40 - Test cases 41 - Bug content can potentially be discussed on public IRC/Slack channels 42 and mailing list emails. 43 44 When patching for a security bug, you’ll need to be mindful about what 45 type of information you share and where. 46 47 In commit messages 48 ~~~~~~~~~~~~~~~~~~ 49 50 People are watching code check-ins, so we want to avoid sharing 51 information which would disclose or help finding a vulnerability too 52 easily before we shipped the fix to our users. This includes: 53 54 - The **nature of the vulnerability** (overflow, use-after-free, XSS, 55 CSP bypass...) 56 - **Ways to trigger and exploit that vulnerability** 57 - In commit messages, code comments and test cases. 58 - The fact that a bug / commit is security-related: 59 60 - **Trigger words** in the commit message or code comments such as 61 "security", "exploitable", or the nature of a security vulnerability 62 (overflow, use-after-free…) 63 - **Security approver’s name** in a commit message. 64 - The Firefox versions and components affected by the vulnerability. 65 - Patches with an obvious fix. 66 67 In Bugzilla and other public channels 68 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 69 70 In addition to commits, you’ll need to be mindful of not disclosing 71 sensitive information about the bug in public places, such as Bugzilla: 72 73 - Mention the bugs in comment of the private bug instead. 74 - Do not comment sensitive information in public related bugs. 75 - Also be careful about who you give bug access to: **double check 76 before CC’ing the wrong person or alias**. 77 - As of recently, you may now add public bugs in the “duplicate”, 78 “depends on”, “blocks”, “regression”, “regressed by”, or “see also” section. 79 Bugzilla will only reveal those relationships to people with ``editbugs`` 80 permission or access to the security bug. 81 82 On IRC, Slack channels, GitHub issues, mailing lists: If you need to 83 discuss about a security bug, use a private channel (protected with a 84 password or with proper right access management) 85 86 During Development 87 ------------------ 88 89 Testing security bugs 90 ~~~~~~~~~~~~~~~~~~~~~ 91 92 Pushing to Try servers requires Level 1 Commit access but **content 93 viewing is publicly accessible**. 94 95 As much as possible, **do not push to Try servers**. Testing should be 96 done locally before checkin in order to prevent public disclosing of the 97 bug. 98 99 Because of the public visibility, pushing to Try has all the same concerns 100 as committing the patch. Please heed the concerns in the 101 :ref:`landing-your-patch` section before thinking about it, and check with 102 the security team for an informal "sec-approval" before doing so. 103 104 **Do not push the bug's own vulnerability testcase to Try.** 105 106 If you need to push to Try servers, make sure your tests don’t disclose 107 what the vulnerability is about or how to trigger it. Do not mention 108 anywhere it is security related. 109 110 Obfuscating a security patch 111 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 112 113 If your security patch looks obvious because of the code it contains 114 (e.g. a one-line fix), or if you really need to push to Try servers, 115 **consider integrating your security-related patch to non-security work 116 in the same area**. And/or pretend it is related to something else, like 117 some performance improvement or a correctness fix. **Definitely don't 118 include the bug number in the commit message.** This will help making 119 the security issue less easily identifiable. (The absolute ban against 120 "Security through Obscurity" is in relation to cryptographic systems. In 121 other situations you still can't *rely* on obscurity but it can 122 sometimes buy you a little time. In this context we need to get the 123 fixes into the hands of our users faster than attackers can weaponize 124 and deploy attacks and a little extra time can help.) 125 126 Requesting sec-approval 127 ~~~~~~~~~~~~~~~~~~~~~~~ 128 129 See :ref:`Security Bug Approval Process` 130 for more details 131 132 .. _landing-your-patch: 133 134 Landing your patch (with or without sec-approval) 135 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 136 137 Before asking for sec-approval or landing, ensure your patch does not disclose 138 information about the security vulnerability unnecessarily. Specifically: 139 140 #. The patch commit message and its contents should not mention security, 141 security bugs, or sec-approvers. 142 Note that you can alter the commit message directly in phabricator, 143 if that's the only thing you need to do - you don't need to amend your 144 local commit and re-push it. 145 While comprehensive commit messages are generally encouraged; they 146 should be omitted for security bugs and instead be posted in the bug 147 (which will eventually become public.) 148 #. Separate out tests into a separate commit. 149 **Do not land tests when landing the patch. Remember we don’t want 150 to 0-day ourselves!** This includes when pushing to try. 151 152 - Tests should only be checked in later, after an official Firefox 153 release that contains the fix has been live for at least 154 four weeks. For example, if Firefox 53 155 contains a security issue that affects the world and that issue is 156 fixed in 54, tests for this fix should not be checked in 157 until four weeks after 54 goes live. 158 159 The exception to this is if there is a security issue that doesn't 160 affect any release branches, only mozilla-central and/or other 161 development branches. Since the security problem was never 162 released to the world, once the bug is fixed in all affected 163 places, tests can be checked in to the various branches. 164 - There are two main techniques for remembering to check in the 165 tests later: 166 167 a. clone the sec bug into a separate "task" bug **that is also 168 in a security-sensitive group to ensure it's not publicly visible** 169 called something like "land tests for bug xxxxx" and assign to 170 yourself. It should get a "sec-other" keyword rating. 171 172 Tip: In phabricator, you can change the bug linked to 173 a commit with tests if the tests were already separate, while keeping 174 the previously granted review, meaning you can just land the patch 175 when ready, rather than having your reviewer and you have to remember 176 what this was about a month or two down the line. 177 b. Or, set the "in-testsuite" flag to "?", and later set it to "+" 178 when the tests get checked in. 179 180 181 Landing tests 182 ~~~~~~~~~~~~~ 183 184 Tests can be landed **once the release containing fixes has been live 185 at least 4 weeks**. 186 187 The exception is if a security issue has never been shipped in a release 188 build and has been fixed in all development branches. 189 190 Making a security bug public 191 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 192 193 This is the responsibility of the security management team. 194 195 Essentials 196 ---------- 197 198 - **Do not disclose any information about the vulnerability before a 199 release with a fix has gone live for enough time for users to update 200 their software**. 201 202 - This includes code comments, commit messages, tests, public 203 communication channels. 204 205 - If any doubt: '''request sec-approval? ''' 206 - If any doubt: **needinfo security folks**. 207 - **If there’s no rating, assume the worst and treat the bug as 208 sec-critical**. 209 210 Documentation & Contacts 211 ------------------------ 212 213 - :ref:`Normal process for submitting a patch <How to submit a patch>` 214 - `How to file a security bug <https://wiki.mozilla.org/Security/Fileabug>`__ 215 - `Handling Mozilla security bugs (policy) <https://www.mozilla.org/en-US/about/governance/policies/security-group/bugs/>`__ 216 - :ref:`Security Bug Approval Process` 217 - `Contacting the Security team(s) at Mozilla: <https://wiki.mozilla.org/Security>`__