916117771/amlhc

Fork 0

Files

T

916117771 08e56caa72 1

2026-04-21 23:02:15 +08:00

21 KiB

Raw Permalink Blame History

Codebase Concerns

Analysis Date: 2026-04-21

Security

S1. Password Hashing Uses Weak Double-MD5 (HIGH)

Files:

application/common/library/Auth.php line 490-492
application/admin/library/Auth.php line 146-148

Issue: Both frontend and admin password hashing use double-MD5 with salt:

// application/common/library/Auth.php:490-492
public function getEncryptPassword($password, $salt = '')
{
    return md5(md5($password) . $salt);
}

MD5 is cryptographically broken. The inner hash is unsalted, making rainbow table attacks viable. MD5 can be brute-forced at billions of hashes per second on commodity hardware. The salt is only 6 characters (Random::alnum() typically produces short strings).

Impact: If the database is compromised, all user passwords can be cracked rapidly.

Fix approach: Migrate to password_hash() with PASSWORD_BCRYPT or PASSWORD_ARGON2ID. Add a migration script to re-hash passwords on next successful login.

S2. SQL Injection via Raw Queries in Admin Controllers (HIGH)

Files:

application/admin/command/Crud.php lines 440, 444, 462, 466
application/admin/controller/general/Config.php line 293
application/admin/controller/Dashboard.php line 47
application/admin/controller/Command.php line 39
extend/fast/Auth.php line 160

Issue: Multiple raw SQL queries interpolate variables without parameterization:

// application/admin/command/Crud.php:440
$modelTableInfo = $dbconnect->query("SHOW TABLE STATUS LIKE '{$modelTableName}'", [], true);

// application/admin/controller/general/Config.php:293
$tableList = \think\Db::query("SELECT `TABLE_NAME` AS `name`,`TABLE_COMMENT` AS `title` FROM `information_schema`.`TABLES` where `TABLE_SCHEMA` = '{$dbname}';");

// extend/fast/Auth.php:160
->where("aga.uid='{$uid}' and ag.status='normal'")

The Crud.php command receives table names via CLI arguments (--table=) which could be manipulated. The Auth.php interpolates $uid directly into the WHERE clause.

Impact: An attacker with admin access could inject malicious SQL through crafted table names in CRUD generation. The Auth SQL injection is more concerning if $uid can ever be user-controlled.

Fix approach: Use parameterized queries. Validate table names against /^[a-z0-9_]+$/i before embedding in SQL. Replace string interpolation in Auth.php with ->where('aga.uid', $uid)->where('ag.status', 'normal').

S3. HTTP Method Spoofing Enabled (MEDIUM)

Files:

application/config.php line 109

Issue: The config 'var_method' => '_method' allows clients to spoof HTTP methods via POST parameter. An attacker can send _method=DELETE in a POST request to bypass CSRF protections that only check POST requests.

Impact: CSRF bypass via method override on state-changing operations.

Fix approach: Set 'var_method' => '' to disable method spoofing, or ensure all state-changing operations require explicit CSRF token validation regardless of HTTP method.

Files:

application/config.php lines 216-231

Issue: Session cookies lack security flags:

'cookie' => [
    'secure'   => false,  // Not enforced
    'httponly' => '',     // Empty string = disabled
],

Impact: Session cookies vulnerable to interception on HTTP connections and theft via XSS attacks.

Fix approach: Set 'secure' => true (when HTTPS is available) and 'httponly' => true.

S5. Token Key Hardcoded in Config (MEDIUM)

Files:

application/config.php line 264

Issue: The token encryption key is hardcoded directly in the config file:

'token' => [
    'key' => '3byNV4KupeZAvl60sdr2COjDYUmqwPJW',  // line 264
],

This value is committed to git and visible to anyone with repository access. If leaked, an attacker can forge valid tokens.

Impact: Token forgery if the key is exposed.

Fix approach: Move to environment variable via Env::get('token.key') and regenerate the key.

S6. External API Scraping Without Data Validation (HIGH)

Files:

application/index/controller/Index.php lines 20-58

Issue: The get_history() method fetches lottery data from https://history.macaumarksix.com and writes directly to the database with zero validation:

public function get_history()
{
    $client = new \GuzzleHttp\Client();
    $res = $client->request('GET', 'https://history.macaumarksix.com/history/macaujc2/y/2026');
    // ...
    foreach ($data as $item) {
        $insert_data['expect'] = $item['expect'];
        $insert_data['num1'] = $openCode[0];
        // ... direct DB insert without any validation
        Db::name('history')->insert($insert_data);
    }
}

No validation of: response data structure, value ranges (lottery numbers should be 1-49), data types, or expected format of openCode. The year 2026 is hardcoded.

Impact: If the external API is compromised or returns malformed data, the database gets corrupted with invalid lottery records. The hardcoded year requires annual manual updates.

Fix approach: Add data validation (type checks, range validation, schema verification), make the year parameter configurable, and add error handling for API structure changes.

S7. External Scraping Has No Reliability Safeguards (MEDIUM)

Files:

application/index/controller/Index.php

Issue: No retry logic, timeout configuration, or circuit breaker for the external API call. No rate limiting on the get_history endpoint - any user can trigger the scrape repeatedly. No Guzzle timeout is configured.

Impact: External API downtime causes unhandled exceptions. Repeated calls could trigger rate limiting or IP bans on the source API.

Fix approach: Add Guzzle timeout config, implement retry logic with exponential backoff, add rate limiting to the endpoint, and cache results.

Files:

application/config.php line 277

Issue: 'login_captcha' => false disables login CAPTCHA.

Impact: Without login CAPTCHA, the application is vulnerable to brute-force password attacks and credential stuffing. The 10-attempts-per-day lockout is the only defense.

Fix approach: Set 'login_captcha' => true in production.

S9. Upload File Type Check Relies on Client-Supplied MIME (LOW-MEDIUM)

Files:

application/common/library/Upload.php lines 87-98, 106-120

Issue: The upload check uses $this->fileInfo['type'] which comes from the client's Content-Type header. PHP's $_FILES['type'] is set by the browser and can be easily spoofed.

Impact: An attacker could potentially upload files with disguised MIME types if the suffix check has gaps.

Fix approach: Use finfo_open() / finfo_file() to detect actual file type from content bytes.

S10. Suspicious PHP File in Public Directory (HIGH)

Files:

public/ByZjtVrKok.php (1250 bytes, untracked in git)

Issue: A PHP file with a random-looking name exists in the public web root. Any PHP file in public/ is directly executable via HTTP request.

Impact: This could be an unauthorized backdoor, test file, or admin script. If it contains administrative functionality, anyone who discovers the URL could execute it.

Fix approach: Audit the file contents immediately. If not needed, delete it. If it serves a purpose, move it outside the public directory.

S11. No CSRF Protection on API Endpoints (MEDIUM)

Files:

application/api/controller/User.php
application/api/controller/Sms.php
application/api/controller/Ems.php

Issue: API controller methods do not use CSRF token validation. While APIs typically use token-based auth, browser-initiated API calls from authenticated sessions are vulnerable to CSRF.

Impact: An authenticated user visiting a malicious page could have API calls triggered on their behalf (e.g., sending SMS codes, changing account settings).

Fix approach: For browser-initiated API calls that modify state, require CSRF token validation. For pure API usage, ensure token-based auth is mandatory.

S12. 0777 File Permissions (MEDIUM)

Files:

application/common.php line 121
application/admin/command/Addon.php line 271

Issue: @chmod($file, 0777) sets world-writable permissions on created files.

Impact: On shared hosting, any user/process can modify these files, potentially injecting malicious code.

Fix approach: Use 0755 for directories and 0644 for files.

S13. Deprecated mcrypt Fallback (LOW)

Files:

application/common/library/Security.php lines 438-439

Issue: References mcrypt_create_iv() and MCRYPT_DEV_URANDOM — the mcrypt extension was removed in PHP 7.2. The project requires PHP >= 7.4.

Impact: Dead code that will never execute but creates confusion and maintenance debt.

Fix approach: Remove the mcrypt fallback. random_bytes() is sufficient for PHP 7.4+.

S14. exec() Calls with User-Influenced Input (MEDIUM)

Files:

application/admin/command/Crud.php lines 580, 1042, 1232

Issue: Shell commands are constructed with interpolated variables:

exec("php think menu -c {$controllerUrl} -d 1 -f 1");
exec("php think crud -t {$relation['relationTableName']} ...");

Impact: If input variables can be controlled by non-admin users, command injection is possible.

Fix approach: Use escapeshellarg() on all variables interpolated into shell commands.

Maintainability

M1. Massive CRUD Command File (1795 lines)

Files:

application/admin/command/Crud.php (1795 lines)

Issue: This single file handles table analysis, code generation for models, controllers, views, validation, language packs, menu generation, and relation handling. It violates the Single Responsibility Principle.

Impact: Difficult to maintain, test, and extend.

Fix approach: Split into separate classes: TableAnalyzer, ModelGenerator, ControllerGenerator, ViewGenerator, MenuGenerator.

M2. Duplicated Code Across Base Controllers

Files:

application/common/controller/Api.php lines 318-329, 153-160
application/common/controller/Backend.php lines 600-613, 237-244
application/common/controller/Frontend.php lines 149-161, 128-135

Issue: The token() and loadlang() methods are identically duplicated across all three base controllers.

Impact: Changes must be applied in three places.

Fix approach: Move to a shared trait.

M3. Tightly Coupled Backend Traits

Files:

application/admin/library/traits/Backend.php (481 lines)
application/common/controller/Backend.php (614 lines)

Issue: The Backend trait injects massive functionality (index, add, edit, del, recyclebin, restore, destroy, multi, import, selectpage) into every admin controller. The trait and base controller are deeply intertwined.

Impact: All admin controllers carry full CRUD weight. Customizing one method requires overriding the entire trait method.

Fix approach: Split into smaller, focused traits (CrudTrait, ImportTrait, RecycleBinTrait).

M4. No Service Layer

Files: All controllers

Issue: Business logic is embedded directly in controllers. application/index/controller/Index.php handles external API fetching, data parsing, and database insertion all in one method.

Impact: Controllers are difficult to test. Logic cannot be reused across entry points.

Fix approach: Introduce a service layer (e.g., LotteryDataService, UserAuthService).

M5. composer.lock Ignored in Version Control

Files:

.gitignore line 11

Issue: composer.lock is gitignored, meaning dependency versions are not pinned.

Impact: Different environments install different dependency versions, leading to inconsistent behavior and potential security vulnerabilities.

Fix approach: Remove composer.lock from .gitignore and commit it.

Performance

P1. N+1 Query Pattern in Data Scraping (HIGH)

Files:

application/index/controller/Index.php lines 28-45

Issue: For each item from the external API, a separate SELECT + INSERT/UPDATE is executed:

foreach ($data as $item) {
    $exist = Db::name('history')->where('expect', $item['expect'])->find();
    if (!$exist) {
        Db::name('history')->insert($insert_data);
    } else {
        Db::name('history')->where('expect', $item['expect'])->update($insert_data);
    }
}

This generates 2N queries for N records.

Impact: For large datasets, significant database load and slow execution.

Fix approach: Use INSERT ... ON DUPLICATE KEY UPDATE for single-query upsert, or batch operations.

P2. File-Based Cache Only

Files:

application/config.php lines 187-197

Issue: Only file-based caching configured. Token storage defaults to MySQL.

Impact: File I/O is slower than memory-based caching, especially under concurrent access. Token validation on every request adds database load.

Fix approach: Configure Redis for caching and token storage in production.

P3. Dashboard Runs Heavy Queries Without Caching

Files:

application/admin/controller/Dashboard.php lines 24-82

Issue: The dashboard executes ~10+ aggregate queries on every page load with no caching:

$totaluser = User::count();
$todayusersignup = User::whereTime('jointime', 'today')->count();
$sevendau = User::whereTime('jointime|logintime|prevtime', '-7 days')->count();
$dbTableList = Db::query("SHOW TABLE STATUS");

Impact: Dashboard becomes slower as user base grows.

Fix approach: Cache dashboard statistics with a short TTL (e.g., 5 minutes).

P4. No Frontend Asset Build Pipeline

Files:

Gruntfile.js
public/assets/js/backend/command.js
public/assets/js/backend/history.js

Issue: While a Gruntfile.js exists, there is no evidence of automated build pipeline. JavaScript files are loaded individually per controller.

Impact: Slow page load times due to multiple HTTP requests.

Fix approach: Implement asset bundling and minification in the deployment pipeline.

Reliability

R1. Silent Exception Swallowing in Financial Operations (HIGH)

Files:

application/common/model/User.php lines 93-111, 119-137

Issue: The money() and score() methods catch exceptions but only roll back without logging or returning error:

public static function money($money, $user_id, $memo)
{
    Db::startTrans();
    try {
        // ... money operation
        Db::commit();
    } catch (\Exception $e) {
        Db::rollback();
        // No logging, no return value - silent failure
    }
}

Impact: Failed money/score operations silently fail. Financial data integrity is at risk.

Fix approach: Log the exception, throw or return error indicator. Add audit trail for financial operations.

R2. Empty Catch Block in Dashboard

Files:

application/admin/controller/Dashboard.php lines 26-30

Issue: Exception caught and completely ignored:

try {
    \think\Db::execute("SET @@sql_mode='';");
} catch (\Exception $e) {
    // Empty
}

Fix approach: Log the exception at minimum.

R3. No Structured Logging

Files:

application/config.php lines 168-177

Issue: Logging configured minimally with empty level array. No log rotation, no error tracking service.

Impact: Difficult to debug production issues. Log files can grow unbounded.

Fix approach: Configure log levels, add log rotation, integrate with error tracking (e.g., Sentry).

R4. No Validation on History Model

Files:

application/admin/controller/History.php
application/admin/model/History.php
application/admin/validate/History.php (exists but not referenced)

Issue: The History controller has no custom validation. The model has no validation rules. A validate file exists (application/admin/validate/History.php) but is not referenced in the controller.

Impact: Malformed lottery data can be stored through the admin panel.

Fix approach: Enable model validation in the controller and define rules for lottery data fields.

Architecture

A1. ThinkPHP 5.x is Outdated

Files:

composer.json line 19
thinkphp/ directory

Issue: Uses topthink/framework: dev-master (ThinkPHP 5.x fork). ThinkPHP 5 is EOL; current stable is 8.x. The dev-master branch from Gitee is a maintained fork but diverges from the official framework.

Impact: No official security patches. Dependency compatibility issues.

Fix approach: Monitor the FastAdmin fork for security updates. Plan migration to ThinkPHP 6+ when feasible.

A2. Dependency Version Risks

Files:

composer.json

Issue: Unstable and outdated dependencies:

"topthink/framework": "dev-master",       // Unstable branch
"topthink/think-queue": "1.1.6",          // Very old
"topthink/think-captcha": "^1.0.9",       // TP5-era

Impact: dev-master can introduce breaking changes. Old versions may contain known vulnerabilities.

Fix approach: Pin stable versions. Run composer audit regularly.

A3. Framework Code Committed to Repository

Files:

thinkphp/ directory
.gitignore line 2

Issue: The entire ThinkPHP framework source is committed to the repository instead of being managed purely through Composer.

Impact: Repository bloat. Difficulty tracking framework upgrades.

Fix approach: Remove thinkphp/ from the repository and manage solely through Composer.

A4. No API Versioning

Files:

application/api/controller/ (all files)

Issue: API endpoints have no version prefix. Any breaking change affects all clients immediately.

Impact: Inability to make breaking API changes without disrupting existing clients.

Fix approach: Add API versioning (e.g., /api/v1/) using ThinkPHP routes.

Domain-Specific

D1. Lottery Data Accuracy Not Guaranteed (HIGH)

Files:

application/index/controller/Index.php lines 20-58

Issue: Lottery data sourced from a single third-party API with no verification, backup source, or data integrity checks. Raw openCode string is split by comma and mapped directly without range validation.

Impact: If the external source provides incorrect data, the system propagates errors silently.

Fix approach: Add range validation (numbers 1-49), implement backup data sources, and maintain an audit log.

D2. Data Validation Gaps for Lottery Records

Files:

sql/macaujc_history.sql
application/admin/model/History.php
application/admin/validate/History.php

Issue: The History model has no validation rules. The SQL schema has loose VARCHAR constraints without format validation.

Impact: Invalid lottery records can be stored (negative numbers, out-of-range values, malformed dates).

Fix approach: Add validation rules to the History model. Implement data normalization accessors.

D3. Hardcoded Year in Scraping Endpoint

Files:

application/index/controller/Index.php line 23

Issue: The API URL hardcodes the year 2026:

$res = $client->request('GET', 'https://history.macaumarksix.com/history/macaujc2/y/2026');

Impact: Requires annual manual update. After January 1st of a new year, new data won't be fetched.

Fix approach: Make the year dynamic with date('Y') or accept it as a parameter.

Test Coverage Gaps

T1. Zero Automated Tests

Files: Entire application codebase

Issue: No test files exist anywhere in the application directory. No phpunit.xml configuration.

Risk: High

Impact: No automated regression testing. Code changes carry high risk of introducing bugs. Cannot safely refactor.

Fix approach: Set up PHPUnit. Start with unit tests for Auth library, then API endpoints, then lottery data handling.

Additional Concerns

AC1. No .env.example Template

Files:

.gitignore line 15
.env (present)

Issue: .env is gitignored but there is no .env.example template.

Impact: New developers cannot set up the environment without guessing variable names.

Fix approach: Create .env.example with all required variables documented.

AC2. Debug Mode Default Risk

Files:

application/config.php line 21

Issue: 'app_debug' => Env::get('app.debug', false) defaults to false, but if .env is misconfigured in production with debug = true, full stack traces are exposed.

Impact: Detailed error messages with file paths and SQL queries exposed to end users.

Fix approach: Explicitly set app_debug=false in production config. Never rely on defaults.

AC3. Install Script Redirect Logic Still Present

Files:

public/index.php lines 16-19

Issue: The entry point checks for install.lock and redirects to ./install.php if missing. Although public/install.php has been deleted, the redirect logic remains.

Impact: If someone restores install.php, the installation wizard becomes publicly accessible.

Fix approach: Remove the redirect logic from public/index.php after installation is confirmed complete.

Concerns audit: 2026-04-21

21 KiB Raw Permalink Blame History

Codebase Concerns

Security

S1. Password Hashing Uses Weak Double-MD5 (HIGH)

S2. SQL Injection via Raw Queries in Admin Controllers (HIGH)

S3. HTTP Method Spoofing Enabled (MEDIUM)

S4. Cookie Security Flags Not Set (MEDIUM)

S5. Token Key Hardcoded in Config (MEDIUM)

S6. External API Scraping Without Data Validation (HIGH)

S7. External Scraping Has No Reliability Safeguards (MEDIUM)

S8. Login CAPTCHA Disabled by Default (MEDIUM)

S9. Upload File Type Check Relies on Client-Supplied MIME (LOW-MEDIUM)

S10. Suspicious PHP File in Public Directory (HIGH)

S11. No CSRF Protection on API Endpoints (MEDIUM)

S12. 0777 File Permissions (MEDIUM)

S13. Deprecated mcrypt Fallback (LOW)

S14. exec() Calls with User-Influenced Input (MEDIUM)

Maintainability

M1. Massive CRUD Command File (1795 lines)

M2. Duplicated Code Across Base Controllers

M3. Tightly Coupled Backend Traits

M4. No Service Layer

M5. composer.lock Ignored in Version Control

Performance

P1. N+1 Query Pattern in Data Scraping (HIGH)

P2. File-Based Cache Only

P3. Dashboard Runs Heavy Queries Without Caching

P4. No Frontend Asset Build Pipeline

Reliability

R1. Silent Exception Swallowing in Financial Operations (HIGH)

R2. Empty Catch Block in Dashboard

R3. No Structured Logging

R4. No Validation on History Model

Architecture

A1. ThinkPHP 5.x is Outdated

A2. Dependency Version Risks

A3. Framework Code Committed to Repository

A4. No API Versioning

Domain-Specific

D1. Lottery Data Accuracy Not Guaranteed (HIGH)

D2. Data Validation Gaps for Lottery Records

D3. Hardcoded Year in Scraping Endpoint

Test Coverage Gaps

T1. Zero Automated Tests

Additional Concerns

AC1. No .env.example Template

AC2. Debug Mode Default Risk

AC3. Install Script Redirect Logic Still Present

21 KiB

Raw Permalink Blame History