OWASP / OWASP/Nettacker

[BUG] SQLite/APSW retry exhaustion reports abandoned writes as successful

Open Beginner friendly
#1,734 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
5.6k
Forks
1.2k
Avg merge
1d 21h
Merged PRs (30d)
16

Description

Description

In the APSW/SQLite path, submit_logs_to_db() and submit_temp_logs_to_db() return True after all database is locked retries are exhausted, even though the INSERT was rolled back and no COMMIT occurred.

Both functions document their return value as:

True if success otherwise False

The corresponding exhaustion path therefore appears to violate the function contract.

Current behavior

In nettacker/database/db.py, both functions handle apsw.BusyError("database is locked") by rolling back and retrying.

After Config.settings.max_retries attempts are exhausted, execution falls through to:

logger.warn(messages("database_retries_exhausted"))
return True

At this point, the INSERT has never reached send_submit_query() and there is no committed row.

This differs from the other failure paths, which return False.

Why this matters

submit_logs_to_db() writes to scan_events, including the target, module, port, and condition results associated with scan findings.

submit_temp_logs_to_db() writes intermediate state to temp_events, which is used by multi-step module processing.

The production calls in nettacker/core/lib/base.py currently discard the return value from both functions. process_conditions() then continues and reports the event as successful regardless of whether the database write actually succeeded.

This means an exhausted SQLite write can currently be:

  1. Rolled back
  2. Abandoned
  3. Reported as successful by the DB function
  4. Ignored by the caller
  5. Followed by the normal success path

Existing test coverage

There are already deterministic tests for the database is locked exhaustion path.

However, test_apsw_busy_error does not assert the return value, while test_temp_log_busy_error explicitly asserts that the result is truthy after exhaustion:

assert result  # we're continuing operation hence it returns True

This appears to encode the current behavior rather than the documented True if success otherwise False contract.

Proposed fix

I would like to get maintainer feedback on the fix before implementing it.

One option would be to:

  • Return False after retry exhaustion in both functions.
  • Update the affected test expectation.
  • Add an explicit return-value assertion to the existing test_apsw_busy_error.

This would make the return value consistent with the documented contract and the other failure paths, without changing the existing retry count, transaction handling, threading model, or database architecture.

I am intentionally not proposing a database queue or broader retry architecture at this stage.

Question for maintainers

Would returning False on database_retries_exhausted be the preferred contract, while retaining the current "continue the scan" behavior?

If so, I can work on a small regression fix and tests.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in nettacker/database/db.py by tracing the APSW BusyError exhaustion paths in submit_logs_to_db() and submit_temp_logs_to_db(). Run test_apsw_busy_error and test_temp_log_busy_error, then update the exhaustion expectations and add the missing return-value assertion. Done means exhausted writes return False while retry behavior remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, sqlite
Domain
backend, databases
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.