Skip to content

Commit

Permalink
KTOR-4046 Check file descriptor before executing FD_CLR or FD_SET (#4137
Browse files Browse the repository at this point in the history
)

KTOR-4046 Check file descriptor before executing FD_CLR or FD_SET to prevent UB
  • Loading branch information
Stexxe authored and osipxd committed Nov 8, 2024
1 parent 27e55de commit 1a3f83d
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 4 deletions.
4 changes: 4 additions & 0 deletions ktor-network/nix/interop/network.def
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@ void selector_release_fd_set(selection_set set) {
int selector_pselect(int descriptor, selection_set read_set, selection_set write_set, selection_set error_set) {
return pselect(descriptor, read_set.value, write_set.value, error_set.value, NULL, NULL);
}

static inline int fd_setsize() {
return FD_SETSIZE;
}
21 changes: 17 additions & 4 deletions ktor-network/nix/src/io/ktor/network/selector/SelectUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import kotlin.math.*
internal expect fun inetNtopBridge(type: Int, address: CPointer<*>, addressOf: CPointer<*>, size: Int)

@OptIn(InternalAPI::class)
internal class SelectorHelper {
internal class SelectorHelper @OptIn(ExperimentalForeignApi::class) constructor(
private val fdSetSize: Int = fd_setsize()
) {
private val wakeupSignal = SignalPoint()
private val interestQueue = LockFreeMPSCQueue<EventInfo>()
private val closeQueue = LockFreeMPSCQueue<Int>()
Expand All @@ -38,12 +40,16 @@ internal class SelectorHelper {
return false
}

fun start(scope: CoroutineScope) {
scope.launch(CoroutineName("selector")) {
fun start(scope: CoroutineScope): Job {
val job = scope.launch(CoroutineName("selector")) {
selectionLoop()
}.invokeOnCompletion {
}

job.invokeOnCompletion {
cleanup()
}

return job
}

fun requestTermination() {
Expand Down Expand Up @@ -134,6 +140,13 @@ internal class SelectorHelper {
) {
val set = descriptorSetByInterestKind(event, readSet, writeSet)

check(event.descriptor >= 0) {
"File descriptor ${event.descriptor} is negative"
}
check(event.descriptor < fdSetSize) {
"File descriptor ${event.descriptor} is larger or equal to FD_SETSIZE ($fdSetSize)"
}

select_fd_add(event.descriptor, set)
select_fd_add(event.descriptor, errorSet)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
*/

package io.ktor.network.sockets.tests

import io.ktor.network.selector.*
import io.ktor.network.selector.EventInfo
import io.ktor.network.selector.SelectorHelper
import io.ktor.test.dispatcher.*
import kotlinx.coroutines.*
import kotlin.coroutines.*
import kotlin.test.*

class SelectNixTest {
@Test
fun selectDescriptorIsEqualOrLargerThanFdSetSize() = testSuspend {
val scope = CoroutineScope(
CoroutineExceptionHandler { _, cause ->
val regexStr = """File descriptor \d+ is larger or equal to FD_SETSIZE \(3\)"""
val message = cause.message
assertNotNull(message)
assertTrue(
regexStr.toRegex().matches(message),
"Expected message in format \"$regexStr\", got $message"
)
}
)

val selector = SelectorHelper(3)
val job = selector.start(scope)

selector.interest(EventInfo(1, SelectInterest.READ, Continuation(coroutineContext) {}))
selector.interest(EventInfo(2, SelectInterest.READ, Continuation(coroutineContext) {}))
selector.interest(EventInfo(3, SelectInterest.READ, Continuation(coroutineContext) {}))

launch {
delay(1000)
if (scope.isActive) {
selector.requestTermination()
job.cancel()
fail("Exception should have been thrown")
}
}

job.join()
}

@Test
fun selectDescriptorIsNegative() = testSuspend {
val scope = CoroutineScope(
CoroutineExceptionHandler { _, cause ->
assertEquals("File descriptor -1 is negative", cause.message)
}
)

val selector = SelectorHelper()
val job = selector.start(scope)

selector.interest(EventInfo(-1, SelectInterest.READ, Continuation(coroutineContext) {}))

launch {
delay(1000)
if (scope.isActive) {
selector.requestTermination()
job.cancel()
fail("Exception should have been thrown")
}
}

job.join()
}
}

0 comments on commit 1a3f83d

Please sign in to comment.