Replace strcpy() with memcpy() when copying the error message into
the TFTP error packet buffer. The buffer length is already computed
as (4 + strlen(msg) + 1), so memcpy with (len - 4) copies exactly
the right amount. This avoids reliance on strcpy's null-terminator
scanning behavior for a fixed-length copy.
Signed-off-by: OrbisAI Security <mediratta01.pally@gmail.com>
The translate_readwrite() function opens files relative to the
persistent readwrite directory based on client-supplied paths.
A malicious TFTP client could use '..' sequences to escape the
intended directory and read/write arbitrary files on the host.
Add a strstr() check to reject any path containing '..' before
opening the file.
Signed-off-by: OrbisAI Security <mediratta01.pally@gmail.com>
asprintf() can fail, keeping the path_with_zstd_extension unchanged,
which can lead to further errors / crashes. Return early if asprintf
fails.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Some Qualcomm modem firmware (Xiaomi raphael on sm8150, db845c, ...)
requests its protection-domain firmware blobs via TFTP-over-QRTR at
Android-style vendor partition paths, which tqftpserv currently
rejects with "invalid path ... rejecting", leaving the wifi (or other)
protection domain unloaded.
Add two more readonly prefixes that delegate to translate_readonly()
(the existing remoteproc-firmware-dir search):
/readonly/vendor/firmware/ - Xiaomi raphael (sm8150), ...
/readonly/vendor/firmware_mnt/image/ - db845c (per issue #7)
For this to actually serve a blob, the file needs to exist in one of
the firmware directories that translate_readonly() iterates (i.e.
alongside a remoteproc's firmware= attribute). On postmarketOS
xiaomi-raphael, the firmware-xiaomi-raphael-wlan package currently
ships wlanmdsp.mbn at /lib/firmware/wlanmdsp.mbn; once that is moved
(or symlinked) into /lib/firmware/qcom/sm8150/xiaomi/raphael/
(alongside modem.mbn), ath10k_snoc completes its probe, phy0 appears,
and NetworkManager auto-creates wlan0.
Refs: https://github.com/linux-msm/tqftpserv/issues/7
Signed-off-by: Astro Enigma <astronazer@gmail.com>
On qcm6490, the inbuilt modem requests firmware images via TFTP
RRQ using the path prefix /readonly/firmware/modem_pr/so/ instead of the
previously expected /readonly/firmware/image/. The server fails to match
this path and does not invoke translate_readonly, causing the modem boot
to fail.
Add READONLY_FW_BASE and READONLY_MODEM_PATH macros to explicitly match
/readonly/firmware/modem_pr and reuse translate_readonly() by passing
the modem_pr relative path (e.g., modem_pr/so/file.mbn).
translate_readonly() then searches the remoteproc firmware directory
and constructs the full path as:
READONLY_FW_BASE/<fw_dir>/modem_pr/so/file.mbn
With this fix, the modem boots successfully and the link comes up.
Signed-off-by: Bhaskar Valaboju <bhaskar.valaboju@oss.qualcomm.com>
The macro name TQFTPSERV_TMP is misleading since /var/lib/tqftpserv
is persistent storage, not a temporary directory. Rename it to
TQFTPSERV_RW_DIR to reflect its role as the base directory for
/readwrite/ path requests (WRQ operations).
Also update the warn() messages and kdoc comment in
translate_readwrite() to drop the stale "temporary" wording.
Signed-off-by: Bhaskar Valaboju <bhaskar.valaboju@oss.qualcomm.com>
Modem subsystems write calibration data and other persistent files via
TFTP to the readwrite path. These files must survive system reboots to
avoid re-calibration on every boot cycle.
Change TQFTPSERV_TMP from /tmp/tqftpserv to /var/lib/tqftpserv on
non-Android builds. The /tmp directory is typically a tmpfs that is
cleared on reboot, which would cause loss of modem calibration data.
/var/lib/tqftpserv provides persistent storage that survives reboots
and is the appropriate location for application state data.
Update tqftpserv.service.in and include StateDirectory to let systemd
create /var/lib/tqftpserv if it is not present.
Signed-off-by: Bhaskar Valaboju <bhaskar.valaboju@oss.qualcomm.com>
When receiving DATA packets during a WRQ transfer, the payload size
may be smaller than the negotiated blksize. This indicates the final
DATA packet, even if fewer than wsize packets have been received.
ACK such packets immediately instead of waiting for a full window,
so the transfer can complete correctly.
Signed-off-by: Bhaskar Valaboju <bhaskar.valaboju@oss.qualcomm.com>
The current logs are a mishmash of prints to stdout and stderr. This
both presents a bad user experience and doesn't fold well with
integration into syslog or similar systems.
Similar to log_debug(), introduce a log_err() and log_info(). The prior
can be used to indicate errors in the server or events that the
implementer/owner of the system might benefit from paying attention to.
The latter can be used to provide useful insights into the what the
server is doing (e.g. what files does it serve).
The "[TQFTP]" prefix is removed from the strings, as they don't add any
value when routed through any form of logging framework.
Further polishing of the log entries, both in placement and content, is
expected but the baseline is less of a mishmash now.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
There's a few commented out development/debug prints lingering in the
code that can be useful to enable in order to get some basic insights
into what's going on in the server.
While more work is needed to determine what level of debug prints are
useful, and where they should be placed, introduce a '-d' argument to
tqftpserv and translate these to runtime-conditional prints for now.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
While servicing a read, the only valid messages to receive are OP_ACK
and OP_ERROR. The code correctly handles this in terms of cleaning up
resources and closing the socket, but it doesn't inform the client that
the transfer is terminated.
Resolve this by sending out an error message.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The creation of the oath message is mix of strcpy() and sprintf(). While
it won't overflow the fixed size buffer today, it's better to introduce
bounds check for the future, and to avoid the "dangerous" functions.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The server currently accepts any filename from clients without
validation, allowing directory traversal attacks using paths like
'../../../etc/passwd'. This could expose sensitive files outside the
intended serving directory.
Add a sanitize step to validate that the filename does not contain '..'
portions.
Log rejected paths for security auditing.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Replace fstat() with lseek(SEEK_END) for determining file size when
the tsize option is requested. This approach is more portable and
doesn't require struct stat.
Additionally, handle the case where lseek() fails (e.g., on unseekable
files like pipes or special devices). When this occurs, send a TFTP
ERROR with code 0 (Not defined) and a descriptive message to inform
the client that the file size cannot be determined.
After obtaining the size, reset the file position to the beginning
with lseek(SEEK_SET) to ensure subsequent reads start from the correct
offset.
Also fix resource leak: close the socket when file open fails, ensuring
proper cleanup in all error paths.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The current behavior when failing to write out a block of data to the
storage media is to print an error and return, presumably hoping that
once the client retries the "lost" packet there will suddenly be space
in the file system.
Instead communicate the error to the client, as expected.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Most of the error handling in RRQ and WRQ are "handled" by /* XXX */
comments. Implement some actual error handling, to not leak the
resources.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
While we don't actively tear down the client context on failure, when it
happens (as the remote processor or the client socket goes away), the
block buffer object will be freed.
tftp_send_data() freeing the buffer on failure will therefor result in a
double free, so leave it around.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
When pread() fails during an active file transfer, send a TFTP ERROR
packet to inform the client rather than silently aborting. This can
occur due to I/O errors, filesystem issues, or the file being truncated
during transfer.
Per RFC 1350, we use error code 0 (Not defined) with a descriptive
message, as there's no specific error code for read failures during
transfer. The error message now includes strerror(errno) to provide
detailed diagnostic information in the server logs.
This ensures the client receives proper notification of the failure
rather than timing out waiting for data that will never arrive.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Per RFC 1350, when the server cannot create or open a file for writing,
it must send an ERROR packet with error code 2 (Access violation) to
inform the client of the failure.
Previously, when translate_open() failed during a write request, we
would log the error locally but fail to notify the client. This left
the client waiting indefinitely for an acknowledgment that would never
arrive.
Now we properly send an error response using the standard TFTP error
mechanism, allowing the client to handle the failure appropriately
rather than timing out. This could occur due to permission issues,
disk full conditions, or other filesystem errors.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Per RFC 1350, TFTP supports multiple transfer modes (netascii, octet,
mail), but this server only implements octet mode. When a client
requests a different mode, we must send an ERROR packet with error
code 4 (Illegal TFTP operation) rather than silently dropping the
request.
Previously, we would log the rejection but fail to notify the client,
leaving it waiting indefinitely for a response that would never come.
Now we properly inform the client that only octet mode is supported,
allowing it to either retry with the correct mode or report a
meaningful error to the user.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Both handle_rrq() and handle_wrq() functions uses strlen() on the
filename and mode fields from network packets without first verifying
they were NUL-terminated and within the packet bounds. Malformed packets
could cause buffer overruns when these strings were processed.
Replace strlen() with strnlen() and verify both filename and mode
are properly NUL-terminated within the received packet buffer.
Reject malformed packets early with appropriate error logging.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The parse_options() function invokes strlen() without considering that
the incoming options might not be NUL-terminated, uses atoi() without
error checking and does not validate option values against RFC-specified
limits. This allows malicious clients to trick the server to read beyond
the buffer, request invalid values that could cause resource exhaustion
or integer overflow attacks.
Validate that each option is within bounds, then add RFC-compliant
constants for option limits (MIN/MAX_BLKSIZE per RFC 2348, MIN/MAX_WSIZE
per RFC 7440, and reasonable limits for custom options). Replace atoi()
with strtoll() for proper error detection. Validate all option values
and reject requests with out-of-range values, logging the invalid values
for diagnostics.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The tftp_send_error() function is going to be called in many more
places, rather than keeping it in the middle of the code and introduce a
forward declaration, move the function up.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Replace magic numbers with RFC-defined symbolic constants for TFTP
error codes throughout the codebase. This improves code readability
and maintainability by making the error semantics explicit.
Defined constants per RFC 1350 and RFC 2347:
- TFTP_ERROR_ENOENT (1): File not found
- TFTP_ERROR_EACCESS (2): Access violation
- TFTP_ERROR_EBADOP (4): Illegal TFTP operation
- TFTP_ERROR_EOPTNEG (8): Option negotiation failed
The use of symbolic names makes it immediately clear what error
condition is being reported, and ensures consistency across all
error reporting paths in the server.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The buffer type differs in signedness (buffer is unsigned, local
pointer is signed), resulting in build warnings even with default
settings.
Correct the local type to fix the warnings.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
2026-01-23 20:06:10 -06:00
3 changed files with 480 additions and 134 deletions
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Dmitry Baryshkov
OrbisAI Security
Astro Enigma
Bhaskar Valaboju
Yumi Yukimura
Bjorn Andersson