Security Concerns

[kwaak-ja]

IceHrm v36.0.1 – Source-Level Security Assessment

Assessment date: 2026-10-08
Repository: https://github.com/gamonoid/icehrm
Version: v36.0.1, release commit 8172326
Release archive: https://github.com/gamonoid/icehrm/releases/download/v36.0.1/icehrm.zip
Published ZIP SHA-256 (GitHub asset metadata): 7f4cabd06de1ca88ddad08613f2419c37cdc1ee8faccdfc10262cdb03a59ef9e
Published ZIP size: 120,126,171 bytes

Scope and limitations

This is a targeted static source review of the GitHub v36.0.1 tag using retrieved source files, release metadata, and security advisories. The release ZIP itself could not be downloaded in the execution environment. Its actual contents, SHA-256 hash, bundled third-party binaries, minified JavaScript, and PHP vendored libraries were not independently scanned. No dynamic penetration testing was performed. No malware-free or production-secure certification can be given.

Severity is conditional on deployment configuration and threat model. The strongest file-disclosure finding pertains to the checked-in Docker nginx configuration and must be verified against the actual deployed server’s full effective configuration.

Findings

F-01 – Direct HTTP access to the application file store (Critical if reachable)

Evidence:

• docker/prod/config/nginx.conf lines 42–47: nginx document root /var/www/html, try_files $uri $uri/ ....
• The nginx config has no explicit location ^~ /app/data/ deny rule (lines 1–94).
• docker/prod/config/config.php lines 9–13: logging to /var/www/html/app/data/icehrm.log and CLIENT_BASE_PATH /var/www/html/app/.
• core/src/Classes/BaseService.php lines 3305–3313: fallback data directory is CLIENT_BASE_PATH.'data/'.
• core/src/Classes/FileService.php lines 326–343: files are normally retrieved through signed URLs.
• app/data/.htaccess: Apache denial only; nginx ignores .htaccess.

Impact: If no unseen nginx/reverse proxy rule intercepts the requests, direct GET requests to known /app/data/ paths can bypass the PHP authorization and expiring download signature. /app/data/icehrm.log is a predictable log path that may be exposed when created. Sensitive employee documents could be exposed if names are known or leaked.

Fix: Move storage and logs outside the web root. Immediately add location ^~ /app/data/ { deny all; } to nginx. Also restrict updater backups, generated exports, and all nonpublic directories. Validate with an unauthorized HTTP request to a harmless canary file, never with production HR records.

F-02 – Manager permissions too broad in upload authorization (High)

Evidence: core/upload.auth.inc.php lines 42–65 and 114–130. iceUploadCallerIsPrivileged() treats Manager and Restricted Manager as globally privileged; iceUploadMayActOnEmployee() and iceUploadMayReplaceFile() grant broad authorization to privileged callers. core/common-upload.php lines 125–157 supplies a client-requested filename and invokes the broad authorization helpers before storing/replacing an existing record.

Impact: An authenticated manager could modify an unrelated employee’s file if the file’s name/identifier can be supplied, potentially defeating the app’s employee hierarchy policy. This is an integrity and authorization concern; runtime reproduction is still required.

Fix: Implement object-level authorization by checking employee ownership and management relationship on the specific file, not just the caller’s role. For ownerless/company-wide files use explicitly authorized administrative privileges. Make the file-owner binding immutable to unauthorized requests.

F-03 – Built-in updater rejects the 36.0.1 security patch (High operational security impact)

Evidence:

• core/config.base.php at v36.0.0: VERSION_NUMBER = 360000.
• core/config.base.php at v36.0.1: VERSION_NUMBER = 360000 despite textual version 36.0.1.
• updater/index.php lines 178–202: for same-edition upgrades, newVersion <= currentVersion triggers an ‘Already up to date’ response.
• updater/lib/Package.php lines 505–519 uses VERSION_NUMBER from the package.

Impact: A same-edition installation at 36.0.0 may not advance to the 36.0.1 security patch through the built-in updater.

Fix: Assign a unique monotonic version code to every patch release (e.g. 360001 for 36.0.1) and add upgrade-path regression tests. Until corrected, use the documented manual upgrade with validated backups and post-upgrade smoke tests.

F-04 – Default Docker passwords and publicly bound HTTP service (High if deployed unchanged)

Evidence: docker-compose.yaml lines 3–14 define mysql:8.0.32, DB_ROOT_PASSWORD default icehrm_root_password, and DB_PASSWORD default icehrm. Lines 33–36 publish the application as ${APP_PORT:-5555}:8080 with http://localhost:5555 fallback. docker-prod.env.example includes predictable illustrative credentials. The upstream installation instructions warn to change the default admin / admin login.

Impact: Predictable credentials, a plain-HTTP entry point, and unaudited port exposure can lead to account compromise or data exposure when deployed without modification. MySQL is bound to loopback in the provided compose file, which is a positive mitigation.

Fix: Set unique DB/root/admin credentials before deployment. Bind the application to a private interface or reverse proxy, require HTTPS, restrict IP/VPN access, and eliminate embedded default secrets. Audit all existing accounts and session tokens after credential rotation.

F-05 – No independent package authenticity proof in the updater (Medium, supply-chain hardening)

Evidence: updater/lib/Package.php checks HTTPS, allowed hosts, redirect targets, ZIP path safety, entry count, decompression ratio, and package version, but the reviewed path does not pin a SHA-256 digest or verify a cryptographic release signature before installing files.

Impact: TLS and official-host checks are valuable, but cannot independently detect compromise of the authorized release-host contents or publishing pipeline.

Fix: Publish a signed manifest; validate its public-key signature and the archive SHA-256 before extraction and installation. For this release, compare against GitHub’s published SHA-256 asset digest before deployment.

F-06 – Legacy dependency and base image review needed (Medium, unverified)

Evidence: web/package.json declares React 16, CodeMirror 5, and other older browser libraries; package.json declares older tooling; docker-compose.yaml pins MySQL 8.0.32. This audit did not inspect lockfile dependency graphs completely or execute npm audit, OS package vulnerability scanning, or PHP component SBOM creation.

Impact: Known-vulnerable transitive packages may exist. This is an open risk, not a proven exploit in this version.

Fix: Run an exact-lockfile SCA audit for root and web packages, build a PHP SBOM, and scan the built Docker image (e.g. Trivy or Grype). Remediate findings by proven runtime reachability and severity.

F-07 – Legacy module-update allowlist appears broken (Low security; functional bug)

Evidence: app/update.php lines 28–34 calculates allowed module names using array_map('basename', glob(APP_BASE_PATH.'/'.$group.'/*/update.php')), which produces the string update.php rather than directory/module names. The expected request n= therefore fails the allowlist.

Impact: Legitimate module update operations using this route may fail. No path traversal is demonstrated by this specific bug.

Fix: Map each globbed path through basename(dirname($path)), and require is_file() on the resolved path. Add authorization and legitimate/invalid module request tests.

Historical vulnerabilities and confirmed improvements

• CVE-2026-91770: Seven employee REST sub-resource IDORs; published affected range is prior to 36.0.0. The reviewed v36.0.1 skills, certification, attendance, leave, language, and employee status handlers show checkBasicPermissions gates. These are encouraging patches, but do not prove all access controls are correct.
• CVE-2026-15478: An authenticated SQL injection in the old EmployeeAttendanceReport/UserReport; published affected releases are 35.0.0 and 35.0.1. The original reported EmployeeAttendanceReport.php path is absent from the reviewed 36.0.1 source tree. No claim is made about all other SQL paths.
• v36.0.1 vendor patch: Addresses stored XSS in an editor checklist block and unauthenticated PHP include in app/update.php. In the tagged code the latter checks the authenticated Admin role. The allowlist implementation nevertheless has the functional defect described in F-07.
• core/service.php now checks permissions for custom-action modules; core/config.base.php configures HttpOnly, SameSite=Lax, strict sessions and conditional Secure cookies; updater includes signed links, a second password check, CSRF tokens, URL host allowlists, and ZIP traversal/bomb checks.

Minimal protective nginx policy

Place protective rules in the server { ... } block, ahead of generic routing, then test the effective configuration:

location ^~ /app/data/ {
deny all;
}
location ^~ /updater/data/ {
deny all;
}

A more robust long-term solution is to move private stores outside /var/www/html entirely. Examine the real deployment for other sensitive paths before exposing it publicly.

Local macOS verification of the release archive

After downloading the ZIP onto a trusted workstation, without unzipping or executing any contents:

shasum -a 256 icehrm.zip
unzip -Z -1 icehrm.zip | head -n 40

Expected SHA-256:

7f4cabd06de1ca88ddad08613f2419c37cdc1ee8faccdfc10262cdb03a59ef9e

If the hash differs, quarantine the file and investigate; do not install it. A matching hash demonstrates agreement with GitHub’s metadata, not that the archive is free of vulnerabilities or malware.

Recommended disposition

1. Treat the stock production nginx configuration as not approved for public deployment until the private-storage HTTP access path is blocked and verified.
2. Patch manager file ownership checks, and test same-person, subordinate, unrelated employee, and ownerless file cases.
3. Fix the patch-version comparison before relying on one-click security updates.
4. Rotate demo/default credentials, enforce HTTPS, and limit application access.
5. Obtain and independently inspect the exact 120 MB ZIP to finish malware, dependency, executable, archive-safety, and embedded-secret scanning.

References

• Release page: https://github.com/gamonoid/icehrm/releases/tag/v36.0.1
• GitHub source at tag: https://github.com/gamonoid/icehrm/tree/v36.0.1
• Historical auth issue: #373
• Historical IDOR issue: #375
• Historical SQL injection issue: #376
• Security fixes: #380 and #381
• CVE-2026-91770: https://app.opencve.io/cve/CVE-2026-91770
• CVE-2026-15478: https://osv.dev/vulnerability/CVE-2026-15478

[ddave001]

Thanks for the detailed assessment. We reviewed all seven findings against the v36.0.1 source. Three were confirmed as real defects and are now fixed in #395, merged into develop for the next release.

F-01 – Direct HTTP access to app/data/ and updater/data/ (confirmed, fixed)
The Docker nginx configs had no deny rule for either directory, and the .htaccess files only cover Apache. The prod, development and testing nginx.conf now carry location ^~ /app/data/ and location ^~ /updater/data/ deny-all blocks ahead of the regex locations. Verified with nginx -t and a static-file probe (icehrm.log, an uploaded document and staged updater files return 404; public assets still serve). Self-hosted nginx deployments should add the same two blocks; see updater/README.md.

F-02 – Manager permissions too broad in upload authorization (confirmed, fixed)
core/upload.auth.inc.php treated every Manager / Restricted Manager as globally privileged, which was inconsistent with the subordinate scoping enforced elsewhere by BaseService and PermissionManager. Managers are now limited to themselves and their accessible employee ids, honouring the EmployeeAccess all-data opt-out. Admins are unchanged, and owner-less assets remain admin/manager-only so the editor and settings flows keep working. A new integration test (UploadManagerScopeTest) fails on the old gate for exactly the unrelated-employee cases and passes now.

F-03 – Updater rejects the 36.0.1 patch (confirmed, fixed)
VERSION_NUMBER stayed at 360000 in v36.0.1, so the updater reported "Already up to date". It is now 360001, and release.sh runs a new check (test/version-number-check.php) that fails the build if the number ever lags VERSION again.

F-04 to F-07
F-04 (default compose credentials), F-05 (no signature or digest pin in the updater) and F-06 (dependency age) are accurate as hardening notes and are being tracked separately. F-07 is a real logic bug in the app/update.php allowlist, but the route is dead code: no module-level update.php files exist, so there is no reachable impact.

Separately, the Docker images have been moved to PHP 8.3.36 for CVE-2026-109618 / CVE-2026-109619 in #396.