From 057b59495933d6b335674f7a67a302e7cb691a06 Mon Sep 17 00:00:00 2001 From: Aurum Linh <aurum.linh@gmail.com> Date: Fri, 28 Apr 2023 13:48:08 +0300 Subject: [PATCH] Security and accessibility docs (#3350) --- .../contribute-to-documentation.md | 11 ++- docs/references/accessibility.md | 56 +++++++++++ docs/references/security.md | 94 +++++++++++++++++++ 3 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 docs/references/accessibility.md create mode 100644 docs/references/security.md diff --git a/docs/getting-started/contributing/contribute-to-documentation.md b/docs/getting-started/contributing/contribute-to-documentation.md index e3785fd0a..f1f80f110 100644 --- a/docs/getting-started/contributing/contribute-to-documentation.md +++ b/docs/getting-started/contributing/contribute-to-documentation.md @@ -12,18 +12,19 @@ The Hypha Guide welcomes contributors! There are lots of ways to help out, inclu ## Documentation types -This project consists of several distinct documentation types with specific purposes. When proposing new additions to the project please pick the appropriate documentation type. +This project consists of several distinct documentation types with specific purposes, inspired by a documentation framework called [Diátaxis](https://diataxis.fr/). When proposing new additions to the project please pick the appropriate documentation type. -## Developer +### Developer Tutorials -Provides step-by-step information on how to set up a local instance of Hypha. +Provides step-by-step instructions on how to set up a local instance of Hypha. +> Write technical documentation with a new developer in mind. Create a quickstart doc wherever possible. What is the bare minimum information that a user needs to get started with the feature that you are documenting? Start there. -## User Guides +### User Guides Guides are focused on accomplishing a specific task and can assume some level of prerequisite knowledge. There may be multiple approaches to accomplishing a task, and we welcome your input. Other adopters like the [Digital Freedom Fund ](https://apply.hypha.digitalfreedomfund.org/)and [Open Technology Fund](https://guide.opentech.fund/application-platform-guidance) have created platform guides specific to their application processes. -## Discussions +### Discussions Discussions are focused on understanding and information exchange. These explore a specific topic without a specific goal in mind. https://we.hypha.app/ diff --git a/docs/references/accessibility.md b/docs/references/accessibility.md new file mode 100644 index 000000000..9e4d30bbd --- /dev/null +++ b/docs/references/accessibility.md @@ -0,0 +1,56 @@ +In 2019, Accessibility Lab carried out a Web Content Accessibility Guidelines (WCAG) 2.1 Audit of Hypha. This document serves as a summary of the high-impact issues that were highlighted in the report. + +#### Information, structure and relationships +Labels must be programmatically associated with their control - every form input and control needs a label, or an accessible name. The best way to implement this is to use the html `<label>` element, because that’s what it was designed to do. Use `<fieldset>` to semantically group the form fields and use a `<legend>` to provide a semantic group label. + +#### Sensory characteristics +Instructions provided for understanding and operating content do not rely solely on sensory characteristics of components such as shape, size, visual location, orientation, or sound. Using a graphical symbol can make content difficult to comprehend (i.e. search icon without a visible descriptive text). An image with a text alternative can help in this case. + +#### Use of color +Color was used as the only visual means of conveying information, indicating an action, prompting a response, or distinguishing a visual element. As an solution, `a:hover` and `a:focus` styles were changed to make the hover state more visible and not just depend on color. + +#### Visual presentation +It was noted in the report that the colors of user interface components needed to have more contrast with each other. The contrast of text to background (both for the contrast minimum and contrast enhanced) was increased as a result. + +#### Keyboard accessiblity +The report found that almost all functionality of the content is operable through a keyboard interface, but in order to comply with this requirement 100%, `ContentEditable` (text editor) needs to be made accessible with a keyboard. + +#### Name, role, value +For all user interface components (including but not limited to: form elements, links, and components generated by scripts), the name and role can be programmatically determined; states, properties, and values that can be set by the user can be programmatically set, and notification of changes to these items is available to user agents, including assistive technologies. The only issue that was found was with `ContentEditable` (text editor) — issues found with getting to the toolbar with just a keyboard and issues with lacking visual focus. + +#### Navigation +A mechanism is available to bypass blocks of content that are repeated on multiple web pages - a keyboard functional “skip†link should be provided to allow keyboard users to navigate directly to the main content. + +#### Link purpose (in context) +The purpose of each link can be determined from the link text alone or from the link text together with its programmatically determined link context, except where the purpose of the link would be ambiguous to users in general. + +#### Headings and labels +Headings should describe topic or purpose and should not skip hierarchical levels, use headings in proper hierarchical order, (`<h1>` to `<h6>`). The content under a heading relates logically to the heading above it. `<h2>` content relates logically to `<h1>` above it. In this way, the heading structure of a document can be used as a method to organize the documents’ contents hierarchally much like an outline. + +#### Focus visible +This requirement specifies that any keyboard operable user interface has a mode of operation where the keyboard focus indicator is visible. It was highlighted in the report that all links on `a:hover` and `a:focus` need a style that does not only depend on color. This was implemented as a result. + +#### Label in name +For UI components with labels that include images or images of text, the name contains the text that is presented visually. + +#### Target size +The size of the target for pointer inputs is at least 44x44 CSS pixels except in special cases. It was noted that some icons on mobile need to be bigger and more separated to allow for easy use with a finger without risking selecting an adjacent link or button. + +#### Labels and instructions +Labels or instructions need to be provided to help users understand how to complete the form and use individual form controls. It was recommended to display which input fields are required using `aria-required=true`. + +#### Input assistance (error identification) +If an input error is automatically detected, the error is identified and described to the user in text. It was recommended to use `aria-describedby` to programmatically associate the error message with the input field. This was implemented as well as moving the focus to the first field with an error. + +#### Error suggestion +If an input error is automatically detected and suggestions for correction are known, then the suggestions are provided to the user, unless it would jeopardize the security or purpose of the content. + +#### Error prevention +For web pages that require the user to submit information, at least one of the following is true: + +- Reversible- submissions are reversible +- Checked- data entered by the user is checked for input errors and the user is provided an opportunity to correct them +- Confirmed- a mechanism is available for reviewing, confirming, and correcting information before finalizing the submission + +#### Parsing +For content written in markup languages, the content needs to be parsed into a data structure. diff --git a/docs/references/security.md b/docs/references/security.md new file mode 100644 index 000000000..456aff710 --- /dev/null +++ b/docs/references/security.md @@ -0,0 +1,94 @@ +In 2021, Radically Open Security carried out a penetration test for the Hypha web app and its user roles. This test resulted in 1 elevated, 5 moderate and 13 low-severity issues. This document outlines the details of the findings and their implemented solutions. + +#### Elevated Threat Level Issues + +1. Several form fields that use TinyMCE allow the input of dangerous characters resulting in XSS when editing a form. + - This would allow an unauthenticated or low privileged user to send a malicious XSS payload (e.g. containing session hijacking, credential stealing, malware) to high privileged users (e.g. staff members and admins). This could result in gaining access to high privileged accounts which would lead to accessing restricted data. +> Update: This issue was resolved in PR [#2495](https://github.com/HyphaApp/hypha/pull/2495) which updated TinyMCE to newer version, PR [#2508](https://github.com/HyphaApp/hypha/pull/2508), using the invalid elements attribute of TinyMCE to validate input, and PR [#2518](https://github.com/HyphaApp/hypha/pull/2518) which disabled the TinyMCE `contextmenu`. + + + +#### Moderate Threat Level Issues + +1. `opentech.fund` and `apply.opentech.fund` accept connections encrypted using TLS 1.0 and/or TLS 1.1. TLS 1.0 has a number of cryptographic design flaws. Modern implementations of TLS 1.0 mitigate these problems, but newer versions of TLS (TLS 1.2) are designed against these flaws and should be used whenever possible. + - Accepting TLS 1.0 and TLS 1.1 makes the data in transit vulnerable to attacks in which an attacker can capture the +encrypted data and decrypt it. +> Update: This issue was resolved by disabling TLS 1.0 and 1.1 + + +2. `opentech.fund` and `apply.opentech.fund` support insecure 3DES ciphers. + - An attacker with a MitM (Machine in the Middle) position can potentially capture and intercept communication between server and clients. + > Update: This issue was resolved by disabling the use of the insecure 3DES ciphers. + + +3. There are no additional authentication checks, such as requiring a password or two-factor token, preventing logged in users from changing their email address. Email addresses are used for account recovery operations that can be abused by attackers. + - An attacker who gains temporary access to a victim's account (be it by exploiting a different vulnerability or by gaining physical access to the victim's machine, a common scenario in office settings) can change the victim's email address to a different address controlled by the attacker, enabling them to take full control of the victim's account by using the forgot password functionality. +> Update: Resolved with PR [#2653](https://github.com/HyphaApp/hypha/pull/2653) ensuring the current password or a two-factor authentication token is required whenever a user attempts to change their email address. + + +4. Two-factor authentication (2FA) can be disabled without providing the current password. + - This could allow an adversary to disable the user's 2FA, for instance by using a XSS attack or other attack. +> Update: Resolved with PR [#2661](https://github.com/HyphaApp/hypha/pull/2661) requiring the user to provide their current password or token before 2FA can be disabled to add an additional layer of security. + + +5. The application incorrectly validates input that can affect the control flow or data flow of a program + - Allowing dangerous input could lead to XSS +> Update: Resolved with PR [#2508](https://github.com/HyphaApp/hypha/pull/2508 ) using invalid elements attribute of TinyMCE to validate input. + + +#### Low Threat Level Issues + +1. Obsoleted CBC ciphers + - TLS misconfiguration - `apply.opentech.fund` are configured to support cipher block chaining encryption (CBC) +> Status: At the time of the report, unresolved. + +2. Open redirect + - Subscribe newsletter is vulnerable to open redirection. +> Recommendation: Do not use user input for URLs. If dynamic URLs are required, use whitelisting. + +3. Insecure password reset + - The password reset functionality is by default set to 8 days and the reset token remains the same until it has been changed. The link does change after the password (including the same password) has been reset. +> Update: Configured password reset timeout to a maximum of 1 hour. + +4. Lack of anti-automation + - Someone can maliciously automate use of functionality such as password reset, 2FA, 2FA backup login, newsletter subscription, apply forms and user login. 200 password requests were issued within 5 seconds and resulted in a flooded mailbox. +> Suggestion: Apply anti-automation on those features. One common way is implementing a Captcha and only show and enforce after a certain amount of requests per IP. + +5. XSS in footer + - The footer incorrectly validates input that results in cross-site scripting (XSS). This XSS can only be created and triggered by high-privileged users (staff and admin), which makes it low impact. However it is recommended to not allow XSS in the first place, since a successful attack could lead to session hijack, credential stealing, or infecting systems with malware. + + +6. Low-privileged user able to purge CDN and cache + - Staff members (high-privileged users), editors and moderators do not see the purge CDN and cache functionality in the UI but are still able to use the functionality by using the following URLs: + - `http://apply.hypha.test:8090/admin/cache/` + - `http://apply.hypha.test:8090/admin/purge/` + +7. XSS in `Used By` + - The `Used By` field incorrectly validates input that results in XSS. This XSS can only be created and triggered by high-privileged users (staff and admin), which makes it low impact. However it is recommended to not allow XSS in the first place, since a successful attack could lead to session hijack, credential stealing, or infecting systems with malware. + + +8. XSS in Reviewer Role + - This XSS can only be created and triggered by high-privileged users (staff and admin), which makes it low impact. However it is recommended to not allow XSS in the first place, since a successful attack could lead to session hijack, credential stealing, or infecting systems with malware. + + +9. User Enumeration with Email Address Change + - Valid users can be found by abusing the Profile change email address functionality. +> Recommendation: Modify the functionality to return only a generic response making it impossible to distinguish between a valid username and invalid username and implement a Captcha. + + +10. XSS in Review Form + - This XSS can only be created and triggered by high-privileged users (staff and admin), which makes it low impact. However it is recommended to not allow XSS in the first place, since a successful attack could lead to session hijack, credential stealing, or infecting systems with malware. + + +11. Django `SECRET_KEY` not random + - `SECRET_KEY` is hardcoded and using a default value. The secret key is used for all sessions if you are using any other session backend than `django.contrib.sessions.backends.cache`, or are using the default `get_session_auth_hash()`, all messages if you are using `CookieStorage` or `FallbackStorage`, all `PasswordResetView` tokens, and any usage of cryptographic signing, unless a different key is provided. +> Recommendation: Automatically generate Strong Random Secret key instead of using a static key. + + +12. Arbitrary document file upload + - No restrictions configured on the document file upload functionality. `.exe` files were successfully uploaded via the document upload functionality. +> Recommendation: Verify all upload functionality. + + +13. Outdated packages are in use + - `npm audit report` — found outdated packages which contain known vulnerabilities. Marked as low because it appears that no functionality is used in the current code that could exploit any of the vulnerabilities. \ No newline at end of file -- GitLab