# Framework Core Entities — Deep Analysis

**Date:** 2026-03-02
**Path:** `GB5Shared/GenerateAutoNumber/`, `GB5Shared/PubSub/OutBox/`, `GB5Shared/Query/`, `GB5Framework/FrameworkBLL/`
**Entities:** AutoNumber, CodeGeneration, FormulaEngine, OutBox, Scheduler/Quartz, CriteriaEngine, ParameterBLL
**Risk Level:** 🔴 CRITICAL — Duplicate auto-numbers under load, SQL injection in formula engine, arbitrary SQL execution

---

## CRITICAL ISSUES

### CORE-01: SQL Injection in AutoNumber Generation — CRITICAL

**File:** `GB5Shared/GenerateAutoNumber/AutoNumber.cs:142, 152`

```csharp
// Line 142 — SELECT with string interpolation
string sql = $"SELECT AUTOID FROM MAUTONUMBER WHERE EntityCode = '{EntityCode}'";
var tempValue = await _QueryExecutor.QueryAsync<int>(LoginDTO, sql);

// Line 152 — UPDATE with string interpolation
string updateSql = $"UPDATE MAUTONUMBER SET AutoId = {currentAutoId + noOfId} WHERE EntityCode = '{EntityCode}'";
await _QueryExecutor.ExecuteAsync(LoginDTO, updateSql);
```

**Impact:** `EntityCode` is passed from entity save operations. If an attacker can control the EntityCode value (via API or DB configuration), full SQL injection is possible:
```
EntityCode = "'; DROP TABLE MAUTONUMBER; --"
→ SELECT AUTOID FROM MAUTONUMBER WHERE EntityCode = ''; DROP TABLE MAUTONUMBER; --'
→ MAUTONUMBER table deleted → ALL auto-numbering fails → system-wide outage
```

**Fix:**
```csharp
// Always use parameterized queries
const string selectSql = "SELECT AUTOID FROM MAUTONUMBER WHERE EntityCode = @EntityCode";
var tempValue = await _QueryExecutor.QueryAsync<int>(LoginDTO, selectSql, new { entitycode = EntityCode });

const string updateSql = "UPDATE MAUTONUMBER SET AutoId = @NewId WHERE EntityCode = @EntityCode";
await _QueryExecutor.ExecuteAsync(LoginDTO, updateSql, new { NewId = currentAutoId + noOfId, EntityCode });
```

---

### CORE-02: AutoNumber Race Condition — Duplicate Numbers Under Concurrency — CRITICAL

**File:** `GB5Shared/GenerateAutoNumber/AutoNumber.cs:184–193`

**Problem:** SELECT then UPDATE is not atomic. Two concurrent transactions read the same `AutoId` value:

```csharp
// Thread A and Thread B both execute this simultaneously:
var currentList = await _QueryExecutor.QueryAsync<int>(LoginDTO, selectSql, ...); // Both read AutoId = 1000
int currentAutoId = currentList.First();                                           // Both see 1000

// RACE WINDOW — both calculate next = 1001

await _QueryExecutor.ExecuteAsync(LoginDTO, updateSql, new { NewId = 1001, EntityCode }); // Both write 1001!
// Result: Two records created with AutoId = 1000 (DUPLICATE!)
```

**Proof scenario:**
```
T=0ms: Thread A:  SELECT AutoId → 1000
T=1ms: Thread B:  SELECT AutoId → 1000 (Thread A hasn't updated yet)
T=5ms: Thread A:  UPDATE AutoId = 1001
T=6ms: Thread B:  UPDATE AutoId = 1001 (same value!)
T=10ms Thread A:  Invoice #1000 created
T=10ms Thread B:  Invoice #1000 created — DUPLICATE INVOICE NUMBER
```

**Impact:** Every entity that uses AutoNumber (invoices, purchase orders, receipts, vouchers) can have duplicate numbers under concurrent load. The probability increases with traffic. In production with 50 concurrent users, this will occur regularly.

**Fix — Atomic increment using OUTPUT (SQL Server):**
```sql
-- Single atomic operation: UPDATE + return new value
UPDATE MAUTONUMBER
SET AutoId = AutoId + @NoOfIds
OUTPUT INSERTED.AutoId
WHERE EntityCode = @EntityCode;
-- Returns the NEW value after increment — no race condition possible
```

```csharp
public async Task<int> GetNextAutoNumber(string entityCode, int noOfIds, LoginDTO login, DbTransaction? tx = null)
{
    const string atomicSql = @"
        UPDATE MAUTONUMBER
        SET AutoId = AutoId + @NoOfIds
        OUTPUT INSERTED.AutoId
        WHERE EntityCode = @EntityCode";

    var newValue = await _queryExecutor.ExecuteScalarAsync<int>(
        login, atomicSql, new { EntityCode = entityCode, NoOfIds = noOfIds }, tx);

    return newValue - noOfIds + 1; // Start of the allocated range
}
```

---

### CORE-03: AutoNumber NOT in Same Transaction as Entity Insert — ACID Violation — CRITICAL

**File:** `GB5Shared/GenerateAutoNumber/AutoNumber.cs`

**Problem:** The auto-number is incremented in a separate transaction from the entity being inserted:

```
1. GetAutoNumber() → BEGIN TX → UPDATE MAUTONUMBER (AutoId = 1001) → COMMIT ✓
   (Number 1001 is now "consumed" even if entity insert hasn't happened)
2. INSERT Invoice (InvoiceNo = 1001) → might FAIL

→ Number 1001 is lost forever (gap in sequence)
→ Manual call to RollbackAutoNumber() required (but callers often forget)
```

**Fix — Pass transaction through:**
```csharp
// Caller must pass its own transaction:
await using var scope = await _queryExecutor.BeginTransactionAsync(login);
try
{
    // AutoNumber and entity INSERT in SAME transaction
    var number = await _autoNumber.GetNextAutoNumber("INVOICE", 1, login, scope.Transaction);
    await _invoiceDAL.InsertInvoice(invoice with { InvoiceNo = number }, login, scope.Transaction);
    await scope.Transaction.CommitAsync();
}
catch
{
    await scope.Transaction.RollbackAsync(); // Auto-number rolled back too!
    throw;
}
```

---

### CORE-04: Formula Engine — Arbitrary SQL Execution — CRITICAL

**File:** `GB5Shared/GenerateAutoNumber/CodeGeneration.cs:415–446`

**Problem:** Formula expressions are loaded from `MFORMULA` table and executed as raw SQL:

```csharp
if (formula.FormulaType == 2) // Expression-based
{
    expression = formula.Expression; // TAKEN DIRECTLY FROM DATABASE
}

string selectSql = loginDto.DatabaseType == 0
    ? $"SELECT {expression} AS EntityId"
    : $"SELECT {expression} AS EntityId FROM dual";

var results = await QueryListAsync<EntityDTO>(loginDto, selectSql, null); // EXECUTED!
```

**Exploit:** An admin-level user (or anyone with DB write access) inserts into `MFORMULA`:
```sql
INSERT INTO MFORMULA (FormulaCode, Expression, FormulaType)
VALUES ('EVIL', '(SELECT TOP 1 PASSWORD FROM MUSER)', 2);
```
Next time CodeGeneration runs for any entity using this formula:
```sql
SELECT (SELECT TOP 1 PASSWORD FROM MUSER) AS EntityId
-- Returns a user password hash — complete credential theft!
```

**Worse case:**
```sql
Expression = "(SELECT 1); EXEC xp_cmdshell('net user hacker P@ss /add'); --"
-- Remote code execution on SQL Server if xp_cmdshell enabled
```

**Fix — Whitelist-only expression evaluation:**
```csharp
private static readonly IReadOnlySet<string> AllowedFunctions = new HashSet<string>(
    StringComparer.OrdinalIgnoreCase)
{ "COUNT", "SUM", "MAX", "MIN", "AVG", "ISNULL", "NVL", "COALESCE", "YEAR", "MONTH" };

private async Task<int> EvaluateFormulaAsync(dynamic formula, LoginDTO loginDto)
{
    if (formula.FormulaType == 2) // Expression
    {
        ValidateExpression(formula.Expression); // Throws if unsafe
    }
    // Proceed only if validation passes
}

private static void ValidateExpression(string expression)
{
    // Only allow: simple column references, allowed functions, literals
    if (Regex.IsMatch(expression, @"(;|DROP|DELETE|INSERT|UPDATE|EXEC|xp_|sp_)", RegexOptions.IgnoreCase))
        throw new SecurityException($"Formula expression contains prohibited content: {expression}");

    // Better: use a proper expression parser/grammar
}
```

---

### CORE-05: SQL Injection in CodeGeneration.UpdateLastNumberAsync — CRITICAL

**File:** `GB5Shared/GenerateAutoNumber/CodeGeneration.cs:408`

```csharp
string updateSql = $"UPDATE MCODEDEFINE SET CATLASTNO = {newLastNumber} WHERE CODEDEFINEID = {codeDefineId}";
await ExecuteNonQueryAsync(loginDto, updateSql);
```

**Fix:**
```csharp
const string updateSql = "UPDATE MCODEDEFINE SET CATLASTNO = @NewLastNumber WHERE CODEDEFINEID = @CodeDefineId";
await ExecuteNonQueryAsync(loginDto, updateSql, new { NewLastNumber = newLastNumber, CodeDefineId = codeDefineId });
```

---

### CORE-06: OutBox — Dapr Unavailability Causes Permanent Message Loss — HIGH

**File:** `GB5Shared/PubSub/OutBox/OutBox.cs:56–114`

```csharp
try
{
    await _dapr.PublishEventAsync(pubsubName, topic, JsonSerializer.Deserialize<object>(evt.PAYLOAD));
    await _queryExecutor.ExecuteAsync(login, OutBoxQB.MarkPublished, ...);
}
catch (Exception)
{
    await _queryExecutor.ExecuteAsync(login, OutBoxQB.MarkFailed, ...); // PERMANENT FAILURE
    // No retry — event is dead after 1 attempt
}
```

**Impact:** Dapr sidecar restart, network blip, or RabbitMQ restart → ALL outbox messages processed in that window are permanently marked FAILED. Business operations never complete end-to-end.

**Fix — Exponential backoff retry:**
```csharp
catch (Exception ex)
{
    _logger.LogError(ex, "Failed to publish outbox event {EventId}", evt.OUTBOXID);
    int retryCount = evt.RETRYCOUNT ?? 0;

    if (retryCount < 3)
    {
        // Retry with exponential backoff — don't mark FAILED yet
        await _queryExecutor.ExecuteAsync(login, OutBoxQB.MarkForRetry,
            new { OutboxId = evt.OUTBOXID, NewRetryCount = retryCount + 1,
                  NextRetryAt = DateTime.UtcNow.AddSeconds(Math.Pow(2, retryCount) * 5) });
    }
    else
    {
        // Dead letter after max retries
        _logger.LogCritical("Outbox event {EventId} moved to dead letter after {Retries} attempts",
            evt.OUTBOXID, retryCount);
        await _queryExecutor.ExecuteAsync(login, OutBoxQB.MarkDeadLetter,
            new { OutboxId = evt.OUTBOXID });
    }
}
```

---

## HIGH SEVERITY ISSUES

### CORE-07: OutBox Idempotency — Same Event Published Twice on Crash

**File:** `GB5Shared/PubSub/OutBox/OutBox.cs:82–88`

```
1. PublishEventAsync(dapr) → SUCCESS
2. UPDATE status = PUBLISHED → CRASH (connection lost)
3. On recovery: outbox re-processes same event (status still PENDING)
4. PublishEventAsync(dapr) → SUCCESS again
5. Consumer processes same event TWICE
```

**Fix:** Include a correlation/idempotency key in the published message:
```csharp
var message = new
{
    IdempotencyKey = evt.OUTBOXID.ToString(), // Stable across retries
    Payload = JsonSerializer.Deserialize<object>(evt.PAYLOAD),
    PublishedAt = DateTime.UtcNow
};
await _dapr.PublishEventAsync(pubsubName, topic, message);

// Consumers check: has IdempotencyKey been processed? → skip duplicate
```

---

### CORE-08: Scheduler Duplicate Job Pickup — Multi-Instance Race

**File:** `GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs:72–110`

Two scheduler instances (pods) both `SELECT` ready jobs before either marks them `InProgress`:

```
Pod-A: LoadReadyTasks → [JobId=5, JobId=7, JobId=9]
Pod-B: LoadReadyTasks → [JobId=5, JobId=7, JobId=9] (same list!)
Both: Publish all 3 jobs to message queue → each job processed TWICE
```

**`[DisallowConcurrentExecution]`** on `SchedulerQuartzJob` only prevents same-pod concurrency, not multi-pod.

**Fix — Atomic job claim (SQL Server):**
```sql
UPDATE TOP(@BatchSize) TJOBEXECUTION
SET Status = 2, ClaimedBy = @PodInstanceId, ClaimedAt = GETUTCDATE()
OUTPUT INSERTED.JobId, INSERTED.ClaimedAt
WHERE Status = 1
  AND NextRunOn <= @Now
  AND (ClaimedBy IS NULL OR ClaimedAt < DATEADD(MINUTE, -10, GETUTCDATE())); -- Recover stale claims
```

---

### CORE-09: DLQ Publish Failure Not Handled — Job Lost Completely

**File:** `GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs:104–108`

```csharp
catch (Exception ex)
{
    _logger.LogError(ex, "❌ Failed JobId={JobId}", task.JobId);
    await publisher.PublishAsync("Scheduler.DLQ", task); // This can also throw!
    // If DLQ publish fails → exception propagates, job lost from all tracking
}
```

**Fix:**
```csharp
catch (Exception ex)
{
    _logger.LogError(ex, "❌ Failed JobId={JobId}", task.JobId);
    try
    {
        await publisher.PublishAsync("Scheduler.DLQ", task);
    }
    catch (Exception dlqEx)
    {
        // Last resort: at minimum mark failure in database
        _logger.LogCritical(dlqEx, "DLQ publish also failed for JobId={JobId}", task.JobId);
        await service.MarkJobFailedInDatabaseAsync(task.JobId, login);
    }
}
```

---

### CORE-10: Criteria Query — Cross-Tenant Data Leakage

**File:** `GB5Shared/Query/FrameWork/CriteriaConfig/CriteriaConfigQuery.cs:86`

```sql
WHERE
    a.CriteriaConfigId = b.CriteriaConfigID
    AND b.CRITERIACONFIGSECTIONID = C.CRITERIASECTIONID
    AND a.CRITERIAUSERID = ccuser.userid
    -- AND a.CRITERIACONFIID = @CRITERIACONFIID  ← COMMENTED OUT!
```

The CriteriaConfigId filter is commented out. This returns ALL criteria configs matching the user, potentially across multiple companies/tenants if the user exists in multiple tenants.

**Fix:** Uncomment the filter AND add company/tenant isolation:
```sql
WHERE
    a.CriteriaConfigId = b.CriteriaConfigID
    AND b.CRITERIACONFIGSECTIONID = C.CRITERIASECTIONID
    AND a.CRITERIAUSERID = ccuser.userid
    AND a.CRITERIACONFIID = @CriteriaConfigId   -- Restore this filter
    AND a.COMPANYID = @CompanyId                 -- Add tenant isolation
```

---

### CORE-11: AutoNumber Rollback Is Manual and Unreliable

**File:** `GB5Shared/GenerateAutoNumber/AutoNumber.cs:208–212`

```csharp
public async Task RollbackAutoNumber(string EntityCode, int lastUsedNumber, LoginDTO LoginDTO)
{
    string rollbackSql = @"UPDATE MAUTONUMBER SET AutoId = @LastUsed WHERE EntityCode = @EntityCode";
    await _QueryExecutor.ExecuteAsync(LoginDTO, rollbackSql,
        new { LastUsed = lastUsedNumber - 1, EntityCode = EntityCode });
}
```

**Problems:**
- Must be explicitly called by every caller that uses `GetAutoNumber` and then fails
- If rollback is forgotten (as is common in legacy code), sequence gap is permanent
- If rollback itself fails, sequence is doubly broken
- No audit trail of rollbacks

**Fix:** ACID compliance via same transaction (see CORE-03). Rollback method becomes unnecessary.

---

## MEDIUM SEVERITY ISSUES

### CORE-12: System Parameters Not Cached — DB Hit on Every Request

**File:** `GB5Framework/FrameworkBLL/Parameter/ParameterBLL.cs`

Every call to `GetParameter("MaxFileSize")` or `GetParameter("TaxRate")` hits the database. System parameters are read-only configuration that change rarely. Under load with 100 concurrent users, each making 5 parameter lookups per request = 500 DB queries/second for essentially static data.

**Fix:**
```csharp
private readonly IMemoryCache _cache;

public async Task<string> GetParameter(string paramCode, LoginDTO login)
{
    var cacheKey = $"param:{login.CompanyId}:{paramCode}";
    return await _cache.GetOrCreateAsync(cacheKey, async entry =>
    {
        entry.AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes(15);
        return await _parameterDAL.GetParameter(paramCode, login);
    }) ?? string.Empty;
}
```

---

### CORE-13: Parameter Value Type Not Validated on Save

**File:** `GB5Framework/FrameworkBLL/Parameter/ParameterBLL.cs:44–71`

A parameter declared as `DataType = "Integer"` with value `"abc"` is saved without validation. When downstream code does `int.Parse(paramValue)`, it crashes with `FormatException`.

**Fix:**
```csharp
private static void ValidateParameterValue(string value, string dataType)
{
    bool valid = dataType switch
    {
        "Integer" => int.TryParse(value, out _),
        "Decimal" => decimal.TryParse(value, CultureInfo.InvariantCulture, out _),
        "Boolean" => bool.TryParse(value, out _),
        "Date"    => DateTime.TryParseExact(value, "yyyy-MM-dd", CultureInfo.InvariantCulture, default, out _),
        "String"  => true,
        _ => throw new ArgumentException($"Unknown parameter data type: {dataType}")
    };
    if (!valid) throw new ValidationException($"Value '{value}' is not valid for type '{dataType}'");
}
```

---

### CORE-14: OutBox ROWLOCK Timeout — Messages Delayed After Instance Crash

**File:** `GB5Shared/Query/OutBox/OutBoxQB.cs:40–45`

```sql
SELECT TOP (@BatchSize) *
FROM TOUTBOX WITH (ROWLOCK, READPAST, UPDLOCK)
WHERE STATUS = @Pending
```

`UPDLOCK` holds lock until the transaction completes. If an instance selects rows and then crashes before marking them `PUBLISHED` or `FAILED`, those rows remain locked until SQL Server lock timeout (typically 30s by default). Messages are delayed for 30 seconds.

**Fix:** Add a heartbeat timeout to detect and recover stuck rows:
```sql
-- In SELECT: also recover messages stuck in PROCESSING for more than 5 minutes
WHERE STATUS = @Pending
   OR (STATUS = 'PROCESSING' AND PROCESSINGSTARTEDAT < DATEADD(MINUTE, -5, GETUTCDATE()))
```

---

## Summary Table

| # | Issue | Severity | File | Lines | Priority |
|---|-------|----------|------|-------|----------|
| CORE-01 | SQL injection in AutoNumber SELECT/UPDATE | 🔴 CRITICAL | AutoNumber.cs | 142, 152 | P0 |
| CORE-02 | AutoNumber race condition — duplicate numbers | 🔴 CRITICAL | AutoNumber.cs | 184–193 | P0 |
| CORE-03 | AutoNumber not in same tx as entity insert | 🔴 CRITICAL | AutoNumber.cs | All | P0 |
| CORE-04 | Formula engine — arbitrary SQL execution | 🔴 CRITICAL | CodeGeneration.cs | 415–446 | P0 |
| CORE-05 | SQL injection in UpdateLastNumberAsync | 🔴 CRITICAL | CodeGeneration.cs | 408 | P0 |
| CORE-06 | Dapr down → permanent outbox message loss | 🟠 HIGH | OutBox.cs | 56–114 | P1 |
| CORE-07 | OutBox no idempotency → double processing | 🟠 HIGH | OutBox.cs | 82–88 | P1 |
| CORE-08 | Scheduler duplicate job pickup multi-pod | 🟠 HIGH | SchedulerTaskManager.cs | 72–110 | P1 |
| CORE-09 | DLQ publish failure — job lost | 🟠 HIGH | SchedulerTaskManager.cs | 104–108 | P1 |
| CORE-10 | Criteria query cross-tenant leakage | 🟠 HIGH | CriteriaConfigQuery.cs | 86 | P1 |
| CORE-11 | AutoNumber rollback manual and unreliable | 🟠 HIGH | AutoNumber.cs | 208–212 | P1 |
| CORE-12 | System parameters not cached | 🟡 MEDIUM | ParameterBLL.cs | All | P2 |
| CORE-13 | Parameter value type not validated | 🟡 MEDIUM | ParameterBLL.cs | 44–71 | P2 |
| CORE-14 | OutBox lock timeout after crash | 🟡 MEDIUM | OutBoxQB.cs | 40–45 | P2 |

---

## Remediation Priority

### P0 — Emergency (AutoNumber and Formula Engine Touch ALL Entities)
1. **CORE-01/CORE-05:** Parameterize ALL SQL in AutoNumber and CodeGeneration
2. **CORE-02:** Replace SELECT+UPDATE with atomic `UPDATE ... OUTPUT` for AutoNumber
3. **CORE-03:** Require caller-provided transaction for AutoNumber operations
4. **CORE-04:** Implement formula expression whitelist/sandbox — disable arbitrary expression execution immediately

### P1 — Sprint 1
5. **CORE-06:** Add exponential backoff retry to OutBox publisher
6. **CORE-07:** Add idempotency key to all published messages
7. **CORE-08:** Make scheduler job pickup atomic with `UPDATE ... OUTPUT`
8. **CORE-09:** Wrap DLQ publish in its own try-catch with DB fallback
9. **CORE-10:** Restore commented-out CriteriaConfigId filter + add CompanyId filter
10. **CORE-11:** AutoNumber rollback becomes unnecessary after CORE-03 fix

### P2 — Sprint 2
11. **CORE-12:** Cache system parameters with 15-minute TTL
12. **CORE-13:** Add value type validation on parameter save
13. **CORE-14:** Add stuck-message recovery query to OutBox processor