Skip to content

Security: fix deserialization, timing attack, open redirect, and name bug#55

Open
nicdavidson wants to merge 5 commits intodevelopfrom
2026-04-security-scan
Open

Security: fix deserialization, timing attack, open redirect, and name bug#55
nicdavidson wants to merge 5 commits intodevelopfrom
2026-04-security-scan

Conversation

@nicdavidson
Copy link
Copy Markdown

Summary

  • Critical: Replace unserialize() with json_encode/json_decode for OAuth 1.0 temp credentials — prevents RCE via cache poisoning
  • High: Sanitize exception messages in OAuth redirect URLs — prevents internal detail leakage
  • High: Default-deny redirect when no domain whitelist configured — prevents open redirect with token theft
  • Medium: Use hash_equals() for Heroku SSO token comparison — prevents timing attack
  • Bug: Fix || to ?: for user name extraction — was storing boolean true ("1") as the user's name in both BaseOAuthService and Google service

Not addressed (needs discussion)

  • Heroku SSO auto-admin (is_sys_admin = true) — business logic decision
  • Session token in URL query string — requires frontend changes
  • OAuth 1.0 fixed cache key race condition — needs broader refactor
  • PKCE support — feature addition

Test plan

  • PHP syntax clean on all 5 changed files
  • No behavioral change to OAuth 2.0 happy path
  • Redirect validation now defaults to same-host when no whitelist

thekevinm and others added 5 commits January 23, 2026 00:10
Added standard overview describing DreamFactory as a secure, self-hosted
enterprise data access platform for enterprise apps and on-prem LLMs.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… bug

- Replace unserialize() with json_encode/json_decode for OAuth 1.0
  temporary credentials (prevents RCE via cache poisoning)
- Use hash_equals() for Heroku SSO token comparison (prevents timing attack)
- Sanitize exception messages in OAuth redirect URLs (prevents info leak)
- Default-deny redirect when no whitelist configured (prevents open redirect)
- Fix || to ?: for user name extraction (was storing boolean "1" as name)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants