fix+add: heap buffer allocation and reliable recv loop in handle_connection + Self-Service paste deletion on port 9998#127
Open
Leproide wants to merge 3 commits into
Conversation
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fiche Bug Fixes
Bug 1 Stack overflow with large buffer sizes
Location
handle_connection()fiche.cRoot cause
The receive buffer was declared as a Variable Length Array (VLA) on the thread stack:
The default value of
buffer_lenis already 32 768 bytes (32 KB).When a larger value is passed via
-B(e.g. 10 MB), the thread stack overflowsimmediately on entry, before any data is received. The crash is silent: the OS
kills the thread with no error message, no URL is returned, and no file is
created.
VLAs on the stack are also flagged as undefined behaviour by
-Wpedanticwhenthe size is not a compile-time constant.
Fix
Replaced the VLA with a heap allocation via
calloc, with proper cleanup onevery exit path:
Bug 2 Connections silently dropped with partial data
Location
handle_connection()fiche.cRoot cause
The original code combined
MSG_WAITALLwithSO_RCVTIMEO:MSG_WAITALLinstructs the kernel to block until exactlybuffer_lenbytesare received. The typical fiche client (
cat file | nc host 9999) sends N byteswhere N <
buffer_lenand then closes the connection.On Linux, when
SO_RCVTIMEOis set and the client closes before filling thebuffer,
recvmay return either:-1witherrno = EAGAIN(timeout fired with partial data pending), or0(client closed the connection cleanly before the timeout).Both cases satisfy
r <= 0, causing the thread to discard all received data,close the socket without responding, and exit as if nothing had been
received. The paste is never saved and the client gets no URL back.
Fix
Removed
MSG_WAITALLand replaced the singlerecvcall with a read loopthat accumulates data until the client closes the connection or the buffer is
full:
The loop exits naturally when the client closes the TCP connection (EOF →
recvreturns 0), regardless of whether the buffer was filled. The 5-secondSO_RCVTIMEOis still in place and still works correctly as a stall guard.The return type was also changed from
inttossize_tto match the returntype of
recv, and the correspondingprintfformat specifier was updated from%dto%zdfor correctness under-Wpedantic.Bug 3 Use-after-free on slug generation failure
Location
handle_connection()fiche.c, inside the slug generation loopRoot cause
In the error path triggered when the slug generation loop exceeds 128 attempts,
the original code called
free(c)beforeclose(c->socket):c->socketis a field of the struct pointed to byc. Reading it afterfree(c)is undefined behaviour, detected by GCC with-Wuse-after-free.Fix
Reordered the cleanup so the socket is closed before the struct is freed:
Summary
handle_connection-Bvalueshandle_connectionMSG_WAITALL+SO_RCVTIMEOhandle_connection