Skip to content

[F-12] fix(network): replace system(dd) with native C in send_tf()#139

Merged
adequatelimited merged 1 commit intoaudit-fixesfrom
audit/F-12-send-tf-dd
Apr 6, 2026
Merged

[F-12] fix(network): replace system(dd) with native C in send_tf()#139
adequatelimited merged 1 commit intoaudit-fixesfrom
audit/F-12-send-tf-dd

Conversation

@adequatelimited
Copy link
Copy Markdown
Collaborator

@adequatelimited adequatelimited commented Apr 5, 2026

Closes #105

Summary

send_tf() constructed a dd shell command with a peer-controlled skip offset and executed it via system(). Replaced with native fseek64/fread/fwrite.

Change

// Before — shell command with peer-controlled parameter
sprintf(cmd, "dd if=tfile.dat of=%s bs=%u skip=%u count=%u 2>/dev/null",
             fname, (int) sizeof(BTRAILER), first, count);
system(cmd);

// After — native file I/O with shutdown awareness
ifp = fopen("tfile.dat", "rb");
fseek64(ifp, (long long) first * sizeof(BTRAILER), SEEK_SET);
ofp = fopen(fname, "wb");
for (j = 0; j < count && Running; j++) {
   if (fread(&bt, sizeof(BTRAILER), 1, ifp) != 1) break;
   if (fwrite(&bt, sizeof(BTRAILER), 1, ofp) != 1) break;
}

Why

  • Eliminates shell process overhead (2 processes per request, up to 37 concurrent connections)
  • Removes peer-controlled values from shell command strings
  • Adds explicit error handling on open/seek/read/write
  • Checks Running flag between iterations for clean shutdown
  • Identical output for all inputs including past-EOF and count=0

Testing

  • Clean build with -Wall -Werror -Wextra -Wpedantic
  • Offline test harness (63 tests): byte-for-byte parity verified between system(dd) and native C across normal operations, edge cases, boundary conditions, permission errors, and real tfile.dat. Posted to issue [F-12] MEDIUM: send_tf() invokes system("dd...") with peer-controlled skip offset — resource abuse #105.
  • Live integration test (25 tests): connected to a running node via OP_TF protocol, received tfile data through the real production code path, compared against direct disk reads — all byte-identical. Covers start/middle/tip/max-count/single/last/partial/EOF scenarios.

Comment thread src/network.c Fixed
@adequatelimited adequatelimited force-pushed the audit/F-12-send-tf-dd branch from 2a2cd21 to cacfa93 Compare April 5, 2026 23:13
Comment thread src/network.c Fixed
Closes #105

send_tf() used sprintf to construct a dd shell command with a
peer-controlled skip offset, then executed it via system(). This
spawned two processes per request with no error checking, and
placed peer-controlled values into a shell command string.

Replaced with fseek64/fread/fwrite operating directly on tfile.dat.
Identical output: reads count trailers starting at offset first,
writes to temp file, sends via send_file(). Handles EOF and
errors explicitly.
@adequatelimited adequatelimited force-pushed the audit/F-12-send-tf-dd branch from cacfa93 to 889c0a6 Compare April 6, 2026 00:14
@adequatelimited adequatelimited merged commit 351cd1b into audit-fixes Apr 6, 2026
4 of 5 checks passed
@adequatelimited adequatelimited deleted the audit/F-12-send-tf-dd branch April 13, 2026 03:24
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.

2 participants