Mobile-Security-Framework-MobSF/AGENTS.md

# MobSF Agent Guidelines

MobSF is a security analysis platform. Every code path processes attacker-supplied
input (APKs, ZIPs, IPAs, manifests) from authenticated but potentially malicious
users. Security must be the default, not an afterthought.

---

## Code Quality — Mandatory Before Every Commit

Run lint and fix all errors before finishing any task:

```bash
tox -e lint
```

Never leave a task with a non-zero exit code from this command.

---

## Security Architecture

Centralized security helpers live in **`mobsf/MobSF/security.py`**. When adding new
security checks, prefer adding them there. Some legacy validators still live in
`mobsf/MobSF/utils.py`; use existing helpers where they are already established.

### Available Security Functions

Import only the helpers needed for the change:

```python
from mobsf.MobSF.security import (
    # Path safety
    is_path_traversal,   # Check raw string for .. sequences, absolute paths, URL encoding tricks
    is_safe_path,        # Containment check after path construction via realpath()

    # Input validation
    is_attack_pattern,   # Detect shell injection: ;, $(), ||, &&
    cmd_injection_check, # Detect OS command injection characters
    is_pipe_or_link,     # Detect symlinks and named FIFOs before reading files

    # Output sanitization
    sanitize_filename,   # Safe filename for Content-Disposition headers
    sanitize_for_logging,# Strip newlines and control chars before logging user input
    sanitize_redirect,   # Allow only relative paths in redirects
    sanitize_svg,        # Strip XSS vectors from SVG content (bleach-based)
    clean_filename,      # Windows-safe filename (unicode normalization)

    # Network / SSRF
    valid_host,          # DNS-resolves host; rejects private/loopback/multicast IPs
)
```

---

## Past Vulnerabilities and Insecure Patterns

Read `.github/SECURITY.md` to understand the full history of security issues in this
codebase. Use it as a guide for what classes of bugs to watch for and what patterns
have been exploited before. When in doubt about whether a pattern is safe, check
whether a similar pattern has appeared in the advisory history.

---

## Incomplete Fix Anti-Pattern

The most common source of security regressions in this codebase is applying a fix to
one code path but not its siblings. Before closing any security fix:

1. Search for all functions or patterns that perform the same operation (e.g., every
   place that resolves an icon path, every place that extracts an archive entry).
2. Verify the fix is applied consistently across **all** of them.
3. Check both the APK binary flow and the source-ZIP flow — they are separate code paths
   with separate callsites and have diverged in the past.

---

## Input Trust Model

- `request.GET` / `request.POST`: untrusted. Validate with forms or explicit checks;
  escape on output.
- File uploads: untrusted. Validate magic bytes, size limits, and extension allowlists.
- Archive entries (`zip`, `tar`, `ar`): untrusted. Check each entry before extraction.
- `AndroidManifest.xml` values: untrusted. Treat as attacker-controlled before using
  them in filesystem operations or rendering them.
- `Info.plist` values: untrusted. Apply the same treatment as manifest values.
- `md5` / `hash` URL parameters: semi-trusted only after validation. Always validate
  with `is_md5()` before using them in paths.
- Device identifiers: untrusted. Use command-injection checks plus format validation.

---

## Django-Specific Security Features

### Form Validation — The Primary Input Sanitization Layer

Prefer Django forms for new request validation. If a view does not use a form, validate
every `request.GET[...]` or `request.POST[...]` value explicitly before using it.

The project uses a mixin composition pattern. Combine the appropriate mixins rather than
writing ad-hoc validation in view code:

```python
# StaticAnalyzer/forms.py — mixins to compose from
AttackDetect   # is_path_traversal + extension allowlist on a 'file' param
APIChecks      # MD5 format check on a 'hash' param (API mode)
WebChecks      # MD5 format check on an 'md5' param (HTML mode)
AndroidChecks  # ChoiceField allowlist for Android scan type
IOSChecks      # ChoiceField allowlist for iOS scan type
```

Custom field validators belong in a `clean_<field>()` method that raises
`forms.ValidationError` on rejection — never return a partial result and check it in
the view. `FormUtil.errors_message(form)` produces the standard error envelope to return
to the caller when `form.is_valid()` is False.

**Use `ChoiceField` for any parameter with a finite set of valid values.** This
eliminates an entire class of injection risk at the form layer with no extra code.
Never use `CharField` and then manually compare the value against an allowlist in the
view — let the form do it.

### View Decorators — Apply All Three

Views that handle sensitive operations should use the applicable Django decorators for
authentication, authorization, and method restriction:

```python
@login_required
@permission_required(Permissions.SCAN)   # or DELETE, SUPPRESS, etc.
@require_http_methods(['POST'])           # or ['GET'] — never omit this
def my_view(request, api=False):
    ...
```

- `@login_required` blocks unauthenticated access.
- `@permission_required` enforces role-based access beyond authentication.
- `@require_http_methods` rejects wrong HTTP verbs before any logic runs,
  preventing CSRF-via-GET and other method-confusion issues.

### Template Auto-Escaping

Django's template engine escapes variables by default. Do **not** use `{% autoescape off %}`
or the `|safe` filter on any value derived from scan data, manifests, or user input.
When rendering user-controlled strings outside of templates (e.g., in a JSON response
built by hand), use `django.utils.html.escape()` explicitly. Template auto-escaping
does not protect JavaScript DOM sinks: never assign scan, task, or user-derived
strings to `innerHTML`; use `textContent` (see `templates/general/tasks.html`).

### ORM — No Raw SQL

Use the Django ORM for all database access. Never use `.raw()` or string-formatted SQL.
When a queryset filter value comes from user input, pass it as a keyword argument
(the ORM parameterizes it automatically):

```python
# Correct
RecentScansDB.objects.filter(MD5=checksum)

# Wrong
RecentScansDB.objects.raw(f'SELECT * FROM ... WHERE MD5 = "{checksum}"')
```

### CSRF

Django's `CsrfViewMiddleware` is enabled globally. Do not use `@csrf_exempt` on any
view that modifies state. API endpoints that accept an `X-Csrftoken` header or use
token-based auth are the only legitimate exception, and that pattern is already
established in the existing API views. Mutating actions (including dynamic-analysis
start/stop/stream) must be POST with a CSRF token, not GET. Split GET-render from
POST-stream when a page both renders and then streams (e.g. logcat).

---

## Archive Extraction Safety

### TAR

Never use a hand-rolled name-only check with `os.path.abspath`. The symlink +
nested-entry combination bypasses it: a symlink member named `escape` passes the
name check, gets extracted to disk, and then a file member named `escape/pwned.txt`
is written through the symlink to an arbitrary location.

`os.path.abspath` normalises `..` but does **not** resolve symlinks.
`os.path.realpath` resolves both — but even `realpath`-based checks that run before
extraction have a TOCTOU window.

Use Python 3.12's built-in filter instead (MobSF requires `python = "^3.12"`):

```python
# Correct — per-member, type-aware, symlink-aware
tar.extractall(dest, members=safe_members_generator, filter='data')

# Wrong — abspath-based name check; blind to symlinks
for member in tar.getmembers():
    if not os.path.abspath(join(dest, member.name)).startswith(dest):
        raise ...
tar.extractall(dest, members=...)
```

`filter='data'` rejects: symlinks outside destination, hardlinks outside destination,
absolute paths, path traversal, and device files — per member, before extraction.

For code that must support Python < 3.12, fall back to: skip all symlink and hardlink
members (`member.issym()` / `member.islnk()`), then use `realpath` for the boundary
check, and validate-then-extract per member rather than batch-validate-then-extractall.

### ZIP

Python's `zipfile` module does not create real filesystem symlinks from Unix symlink
entries — it writes the link target as plain file bytes. The TAR symlink attack does
not apply to ZIP extraction. Use `is_path_traversal` + `is_safe_path` for member name
validation and validate per-member before calling `zip_ref.extract(member, dest)`.

---

## Import Conventions

When adding new imports, maintain alphabetical order within each import group to satisfy
`flake8-import-order`. Group order: stdlib → third-party → Django → local MobSF.

---

## Checklist for Any Change That Touches File I/O or User Input

- [ ] Raw input validated with `is_path_traversal` before path construction
- [ ] Constructed filesystem paths verified with `is_safe_path` when a safe root exists
- [ ] Symlinks and FIFOs rejected with `is_pipe_or_link` before file reads
- [ ] Shell arguments passed as a list, not a formatted string
- [ ] User-controlled strings escaped with `django.utils.html.escape` before rendering
- [ ] SVG content piped through `sanitize_svg`
- [ ] Outbound URLs checked with `valid_host`
- [ ] Redirects wrapped in `sanitize_redirect`
- [ ] Log statements use `sanitize_for_logging` on any user-derived value
- [ ] TAR extraction uses `filter='data'` — not a hand-rolled `abspath` check
- [ ] ZIP extraction validates each member path with `realpath` before `extract()`
- [ ] Every security guard has `continue` / `return` / `raise` — logging alone is not a guard
- [ ] Fix applied symmetrically to all equivalent code paths
- [ ] `tox -e lint` passes with exit code 0