|
| 1 | +# Appsmith CE — Security Review Context |
| 2 | + |
| 3 | +## Public repository disclosure policy |
| 4 | + |
| 5 | +This repository and all pull-request comments are public. |
| 6 | + |
| 7 | +For inline findings, NEVER include: |
| 8 | +- Working exploit code or proof-of-concept payloads |
| 9 | +- Exact HTTP requests, curl commands, malicious URLs, or attacker-controlled IDs |
| 10 | +- Step-by-step reproduction or exploitation instructions |
| 11 | +- Secrets, tokens, credentials, or sensitive production examples |
| 12 | + |
| 13 | +Public comments should state only: |
| 14 | +- The vulnerability class |
| 15 | +- The affected trust boundary and impact at a high level |
| 16 | +- The security invariant that is violated |
| 17 | +- High-level remediation guidance |
| 18 | + |
| 19 | +Keep full reproduction steps, payloads, call traces, and exploit evidence in the |
| 20 | +private Hacktron dashboard only. |
| 21 | + |
| 22 | +For pre-existing or cross-function vulnerabilities not wholly introduced by the |
| 23 | +diff, do not reveal the affected file, function, endpoint, or exploit path in |
| 24 | +the public PR. If a finding cannot be explained safely, publish only a generic |
| 25 | +notice that a potential security issue requires private review. |
| 26 | + |
| 27 | +## Product and trust model |
| 28 | + |
| 29 | +Appsmith is a self-hostable open-source low-code platform for building internal |
| 30 | +tools. |
| 31 | + |
| 32 | +Components: |
| 33 | +- React/TypeScript browser client (`app/client/`) |
| 34 | +- Java 25 Spring WebFlux server (`app/server/`) |
| 35 | +- Node.js RTS real-time server (`app/client/packages/rts/`) |
| 36 | +- Docker, Helm, Caddy reverse proxy, shell scripts (`deploy/`) |
| 37 | + |
| 38 | +## Untrusted inputs |
| 39 | + |
| 40 | +Treat all of these as attacker-controlled: |
| 41 | + |
| 42 | +- Browser requests, headers, and cookies |
| 43 | +- Anonymous app viewers and authenticated editors/workspace members |
| 44 | +- All client-supplied IDs: workspace, application, page, action, datasource, |
| 45 | + environment, branch, artifact, permission group |
| 46 | +- Imported Appsmith applications and their JSON content |
| 47 | +- Git repositories: branches, filenames, file contents, configs, symlinks, |
| 48 | + submodules, archives |
| 49 | +- Uploaded files |
| 50 | +- Datasource and plugin query responses |
| 51 | +- User-authored JavaScript, widgets, templates, URLs, and bindings |
| 52 | +- Admin settings and environment-variable values (application-admin context, |
| 53 | + not deployment-operator configuration) |
| 54 | +- Redirect destinations and DNS responses |
| 55 | + |
| 56 | +A **super-admin** is authorized to administer Appsmith but is NOT trusted to |
| 57 | +obtain container/host command execution, arbitrary filesystem access, cloud |
| 58 | +metadata, or cross-tenant data. Do not suppress findings solely because |
| 59 | +exploitation requires administrator access. |
| 60 | + |
| 61 | +## Authentication and authorization |
| 62 | + |
| 63 | +Authorization is enforced in the **service layer** using `AclPermission` and |
| 64 | +permission helpers (`ApplicationPermission`, `PagePermission`, |
| 65 | +`DatasourcePermission`, `WorkspacePermission`). |
| 66 | + |
| 67 | +Low-level repository methods intentionally omit ACL checks. Do not report a |
| 68 | +repository method merely because it has no permission check — trace whether an |
| 69 | +externally reachable controller/service path performs the correct check before |
| 70 | +calling it. |
| 71 | + |
| 72 | +**Report when:** |
| 73 | +- A service passes `null` or uses a `WithoutPermission` variant without a proven |
| 74 | + earlier authorization check in the same reactive chain |
| 75 | +- A mutation reaches a repository before checking the required permission |
| 76 | +- Read permission is used for a write operation |
| 77 | +- Authorization is performed on one object but the operation uses a different |
| 78 | + client-supplied ID (BOLA/IDOR) |
| 79 | +- Parent-child relationships are not validated (page→application, |
| 80 | + datasource→workspace, action→page, branch→application, environment→workspace) |
| 81 | +- Branch and non-branch code paths enforce different permissions |
| 82 | +- Anonymous viewer access exposes editor-only data, secrets, unpublished state, |
| 83 | + or cross-application resources |
| 84 | + |
| 85 | +For Reactor code: the authorization check must be part of the same subscribed |
| 86 | +`Mono`/`Flux` chain and execute **before** the sensitive operation. Creating a |
| 87 | +permission-checking publisher without chaining or subscribing provides no |
| 88 | +protection. |
| 89 | + |
| 90 | +## Object graphs, mass assignment, and policy mutation |
| 91 | + |
| 92 | +- Treat DTO deep merges, bean copying, JSON conversion, and patch operations as |
| 93 | + security-sensitive. |
| 94 | +- Clients must not overwrite ownership, workspace/application/page/datasource |
| 95 | + relationships, policies, plugin identity, creator identity, or publication |
| 96 | + state unless explicitly authorized. |
| 97 | +- Any operation granting public or anonymous access must verify the caller can |
| 98 | + manage the target object and that every referenced object belongs to the same |
| 99 | + workspace/application. |
| 100 | +- Authorization on one supplied ID does not authorize other IDs nested in the DTO. |
| 101 | + |
| 102 | +## Git and filesystem operations |
| 103 | + |
| 104 | +Git repository content is **attacker-controlled**. The module at |
| 105 | +`app/server/appsmith-git/` handles Git operations. |
| 106 | + |
| 107 | +- All file operations must remain inside the configured Git root or temp directory |
| 108 | +- Lexical `Path.normalize()` alone is insufficient — validate canonical/real paths |
| 109 | +- Account for symlinks, not-yet-created paths, archive entries that escape |
| 110 | + destination, absolute paths, and traversal segments |
| 111 | +- Security validation failures must fail closed |
| 112 | +- Partial clone/import failures must delete residual files |
| 113 | +- For shell execution, verify that every untrusted value remains a single argument |
| 114 | + through the actual shell and command boundary. Escaping is not sufficient when |
| 115 | + escaped fragments are concatenated, decoded again, evaluated twice, or passed |
| 116 | + through another interpreter. `ProcessBuilder` argument lists that do not invoke |
| 117 | + a shell do not require shell escaping. |
| 118 | + |
| 119 | +**High-risk code:** |
| 120 | +- `app/server/appsmith-git/**` |
| 121 | +- Git import/export and autocommit services |
| 122 | +- File and archive helpers |
| 123 | +- Deployment shell scripts |
| 124 | + |
| 125 | +## Environment and command execution |
| 126 | + |
| 127 | +- Never load user-modifiable env files using `source`, `.`, `eval`, or command |
| 128 | + substitution |
| 129 | +- Allowlist environment-variable names; preserve values without shell evaluation |
| 130 | +- Do not pass request, Git, datasource, or environment values through a shell |
| 131 | + without proper escaping |
| 132 | +- Treat `ProcessBuilder`, shell scripts, Docker commands, CI expressions, and |
| 133 | + Caddy configuration as security-sensitive |
| 134 | + |
| 135 | +## Outbound network (SSRF) |
| 136 | + |
| 137 | +`WebClientUtils` and `RestrictedHostFilter` (in `appsmith-interfaces`) are the |
| 138 | +canonical egress controls for WebClient-based plugins (REST, GraphQL, SaaS). |
| 139 | + |
| 140 | +Different connector families use different networking stacks. Do not assume |
| 141 | +WebClient controls cover JDBC, document databases, SMTP, or SSH-tunnel traffic. |
| 142 | +For every changed outbound connector, verify equivalent controls appropriate to |
| 143 | +that transport and deployment mode. |
| 144 | + |
| 145 | +**Report paths that:** |
| 146 | +- Create raw HTTP clients bypassing central egress filtering |
| 147 | +- Reach loopback, link-local, metadata services (169.254.169.254), or IPv6 ULA |
| 148 | +- Validate hostname but not all resolved addresses |
| 149 | +- Are vulnerable to DNS rebinding or redirect-following without revalidation |
| 150 | +- Allow alternate IP encodings or URL parser confusion |
| 151 | +- Let non-HTTP connectors bypass equivalent host validation for their transport |
| 152 | + |
| 153 | +**Do NOT report:** RFC1918 private-address access alone — self-hosted Appsmith |
| 154 | +instances legitimately connect to internal datasources. Only report bypasses of |
| 155 | +an operator-enabled strict-private-address policy. |
| 156 | + |
| 157 | +## Browser, JavaScript, and XSS |
| 158 | + |
| 159 | +Normal React text interpolation (`{}`) is escaped and is NOT an XSS issue. |
| 160 | + |
| 161 | +App editors are expected to author JavaScript for their own applications. Do not |
| 162 | +report that capability itself as XSS. |
| 163 | + |
| 164 | +**Report attacker-controlled data reaching:** |
| 165 | +- `dangerouslySetInnerHTML`, `innerHTML`, or HTML parsers without sanitization |
| 166 | +- Script, custom-widget (`CustomWidget`), worker, iframe, or dynamic eval |
| 167 | + contexts |
| 168 | +- `javascript:`, `data:`, or other executable URL schemes |
| 169 | +- `postMessage` handlers without origin validation |
| 170 | +- Markdown, table HTML, rich text, autocomplete, or error rendering bypassing |
| 171 | + sanitization |
| 172 | +- Cross-application or viewer-to-editor execution boundaries |
| 173 | +- Sandbox escapes, viewer compromise, cross-workspace execution, or secret |
| 174 | + exposure in a privileged Appsmith origin |
| 175 | + |
| 176 | +## Intended datasource behavior |
| 177 | + |
| 178 | +Authorized app editors intentionally write JavaScript, SQL, NoSQL, and plugin |
| 179 | +queries and configure datasource hosts. Do not report this capability alone. |
| 180 | + |
| 181 | +Report injection when lower-privileged viewer input crosses an authorization |
| 182 | +boundary, bypasses an established parameter-binding mechanism, or affects another |
| 183 | +application, workspace, tenant, or privileged Appsmith service. |
| 184 | + |
| 185 | +## Sessions, CSRF, redirects, and trusted origins |
| 186 | + |
| 187 | +- State-changing GET requests combined with browser-managed credentials (cookies, |
| 188 | + basic auth) and missing or insufficient CSRF protection are vulnerabilities. |
| 189 | + Routes protected by non-cookie authentication (API keys, bearer tokens) or |
| 190 | + equivalent controls are not CSRF-vulnerable merely because they accept GET. |
| 191 | +- Review changes to CSRF exemptions, cookie attributes, anonymous endpoints, |
| 192 | + permit-all matchers, login/logout, OAuth state, and session rotation. |
| 193 | +- Origin, Referer, Host, and X-Forwarded-* headers are attacker-controlled unless |
| 194 | + validated against trusted server configuration and trusted proxy boundaries. |
| 195 | +- Token-bearing email links and security redirects must derive their host from |
| 196 | + trusted server configuration. |
| 197 | +- Password reset, verification, and invitation tokens must be single-use, |
| 198 | + time-limited, and absent from logs, analytics, and referrers. |
| 199 | + |
| 200 | +## CI and software supply chain |
| 201 | + |
| 202 | +Pull requests may originate from untrusted forks. |
| 203 | + |
| 204 | +Report: |
| 205 | +- `pull_request_target` workflows that check out or execute PR-controlled code |
| 206 | +- PR-controlled values interpolated into shell commands or GitHub expressions |
| 207 | +- Secrets or write-capable tokens exposed to untrusted jobs |
| 208 | +- Overly broad workflow permissions |
| 209 | +- Untrusted artifact, cache, or workflow-run consumption |
| 210 | +- Mutable third-party action references in privileged workflows |
| 211 | +- Package lifecycle scripts or build hooks introduced by dependency changes |
| 212 | + |
| 213 | +## Multi-tenant caches and asynchronous processing |
| 214 | + |
| 215 | +- Cache keys, background jobs, events, and reactive publishers must preserve |
| 216 | + workspace, organization, application, branch, user, and permission context. |
| 217 | +- Do not reuse authorization-sensitive results across tenants or users. |
| 218 | +- Do not swallow authorization or validation failures with `onErrorResume`, |
| 219 | + `defaultIfEmpty`, or fallback data that permits a failed mutation to proceed. |
| 220 | +- Retries that preserve the original authorization context and propagate the |
| 221 | + error unchanged are acceptable; restrict findings to retries that change |
| 222 | + authorization context or allow a rejected operation to succeed. |
| 223 | + |
| 224 | +## Secrets and sensitive data |
| 225 | + |
| 226 | +Datasource credentials, OAuth tokens, API keys, Git SSH keys, SMTP credentials, |
| 227 | +environment values, session tokens, and encryption material are sensitive. |
| 228 | + |
| 229 | +Review API responses, viewer endpoints, exports, logs, analytics, Redux state, |
| 230 | +error messages, and support bundles for accidental disclosure. |
| 231 | + |
| 232 | +## Noise reduction |
| 233 | + |
| 234 | +- Do not report vulnerabilities in pure documentation, comments, or inert test |
| 235 | + fixture data unless it is executed, shipped, or used by CI with secrets |
| 236 | +- Do not assume test code is harmless — CI scripts, test setup that invokes |
| 237 | + shells, and workflow code remain security-sensitive |
| 238 | +- Do not globally suppress any vulnerability category; use triage feedback for |
| 239 | + recurring safe patterns |
| 240 | +- Do not report that `npm audit` or `yarn audit` advisories exist in lockfiles — |
| 241 | + dependency scanning is handled separately |
0 commit comments