Skip to content

Commit 7800a72

Browse files
riccardoblriccardobl
committed
Merge branch 'fortify' into meman
2 parents 042dcbb + 9094d0e commit 7800a72

15 files changed

Lines changed: 267 additions & 80 deletions

‎build.gradle.kts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ val buildReleaseBinaries = project.findProperty("libdatachannel.build-release-bi
9696
?.ifEmpty { null }
9797
?.toBooleanStrictOrNull()
9898
?: !project.version.toString().endsWith("-SNAPSHOT")
99+
val enableNativeHardening = project.findProperty("libdatachannel.native-hardening")
100+
?.toString()
101+
?.ifEmpty { null }
102+
?.toBooleanStrictOrNull()
103+
?: true
99104

100105
fun DockcrossRunTask.configureSshRemoteBuild(target: BuildTarget) {
101106
if (!ci) {
@@ -157,6 +162,7 @@ fun DockcrossRunTask.baseConfigure(outputTo: Directory, target: BuildTarget) {
157162
extraEnv.put("RELATIVE_PROJECT_PATH", output.get().asFile.toPath().relativize(jniPath.asFile.toPath()).toString())
158163
extraEnv.put("PROJECT_VERSION", project.version.toString())
159164
extraEnv.put("PROJECT_BUILD_TYPE", if (buildReleaseBinaries) "Release" else "Debug")
165+
extraEnv.put("ENABLE_HARDENING", if (enableNativeHardening) "ON" else "OFF")
160166
extraEnv.put("TARGET_FAMILY", target.family)
161167
extraEnv.put("TARGET_CLASSIFIER", target.classifier)
162168
target.image?.let {

‎jni/CMakeLists.txt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ set(CMAKE_C_STANDARD 11)
55
set(CMAKE_CXX_STANDARD 11)
66

77
option(PROJECT_VERSION "The version of the project" "unspecified")
8+
option(ENABLE_HARDENING "Enable compiler/linker hardening flags" ON)
89

910
set(NO_WEBSOCKET ON CACHE BOOL "configure libdatachannel build")
1011
set(NO_MEDIA ON CACHE BOOL "configure libdatachannel build")
@@ -36,6 +37,26 @@ else()
3637
add_compile_options(-g3 -Og)
3738
endif()
3839

40+
if(ENABLE_HARDENING)
41+
if(CMAKE_C_COMPILER_ID MATCHES "GNU|Clang")
42+
add_compile_options(-fstack-protector-strong)
43+
if(NOT APPLE)
44+
if(NOT ANDROID)
45+
if(MINGW)
46+
add_compile_definitions(_FORTIFY_SOURCE=2)
47+
else()
48+
add_compile_definitions(_FORTIFY_SOURCE=3)
49+
endif()
50+
endif()
51+
include(CheckCCompilerFlag)
52+
check_c_compiler_flag("-fcf-protection=full" COMPILER_SUPPORTS_CF_PROTECTION)
53+
if(COMPILER_SUPPORTS_CF_PROTECTION)
54+
add_compile_options(-fcf-protection=full)
55+
endif()
56+
endif()
57+
endif()
58+
endif()
59+
3960
if(APPLE)
4061
add_link_options(-dead_strip)
4162
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
@@ -44,6 +65,9 @@ if(APPLE)
4465
set(CMAKE_MACOSX_RPATH ON)
4566
elseif(NOT WIN32)
4667
add_link_options(-z noexecstack)
68+
if(ENABLE_HARDENING)
69+
add_link_options(-Wl,-z,relro -Wl,-z,now)
70+
endif()
4771
else()
4872
add_link_options(-static)
4973
endif()

‎jni/build.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ cmake_options=(
2727
"-DCMAKE_PROJECT_TOP_LEVEL_INCLUDES=${MOUNT_SOURCE}/jni/cmake-conan/conan_provider.cmake"
2828
"-DPROJECT_VERSION=${PROJECT_VERSION}"
2929
"-DCMAKE_BUILD_TYPE=${PROJECT_BUILD_TYPE}"
30+
"-DENABLE_HARDENING=${ENABLE_HARDENING:-ON}"
3031
)
3132

3233
if [ "$TARGET_FAMILY" = 'android' ]

‎jni/src/callback.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ struct jvm_callback* allocate_callback(JNIEnv* env, jobject callback) {
1111
}
1212
(*env)->GetJavaVM(env, &cb->vm);
1313
cb->instance = (*env)->NewGlobalRef(env, callback);
14+
if (cb->instance == NULL) {
15+
free(cb);
16+
throw_native_exception(env, "Failed to create global callback reference");
17+
return NULL;
18+
}
1419
return cb;
1520
}
1621

‎jni/src/init.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
static JavaVM* global_JVM;
1010
static pthread_key_t thread_key;
11+
static volatile jint jvm_unloading = 0;
1112

1213
void detach_thread() {
1314
JavaVM* jvm = pthread_getspecific(thread_key);
@@ -32,8 +33,8 @@ JNIEnv* get_jni_env_from_jvm(JavaVM* jvm) {
3233
}
3334

3435
JNIEnv* get_jni_env() {
35-
// make sure it's initialized
36-
if (global_JVM == NULL) {
36+
// make sure it's initialized and not in unload phase
37+
if (global_JVM == NULL || jvm_unloading) {
3738
return NULL;
3839
}
3940
return get_jni_env_from_jvm(global_JVM);
@@ -52,6 +53,7 @@ void logger_callback(const rtcLogLevel level, const char* message) {
5253
JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* jvm, void* reserved) {
5354
pthread_key_create(&thread_key, detach_thread);
5455
global_JVM = jvm;
56+
jvm_unloading = 0;
5557
JNIEnv* env = get_jni_env_from_jvm(jvm);
5658
module_OnLoad(env);
5759
rtcInitLogger(RTC_LOG_VERBOSE, &logger_callback);
@@ -60,8 +62,9 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* jvm, void* reserved) {
6062
}
6163

6264
JNIEXPORT void JNICALL JNI_OnUnload(JavaVM* jvm, void* reserved) {
65+
jvm_unloading = 1;
6366
rtcCleanup();
64-
JNIEnv* env = get_jni_env();
67+
JNIEnv* env = get_jni_env_from_jvm(jvm);
6568
module_OnUnload(env);
6669
global_JVM = NULL;
6770
}

‎jni/src/native_channel.c‎

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ SET_CALLBACK_INTERFACE_IMPL(rtcSetErrorCallback, handle_channel_error)
2424

2525
void RTC_API handle_channel_message(const int channelHandle, const char* message, const int size, void* ptr) {
2626
struct jvm_callback* cb = ptr;
27+
if (cb == NULL) return;
2728
JNIEnv* env = get_jni_env();
29+
if (env == NULL) return;
2830
if (size < 0) {
2931
jstring text = (*env)->NewStringUTF(env, message);
3032
call_tel_schich_libdatachannel_PeerConnectionListener_onChannelTextMessage(env, cb->instance, channelHandle, text);
@@ -77,9 +79,18 @@ Java_tel_schich_libdatachannel_LibDataChannelNative_rtcCreateDataChannelEx(JNIEn
7779
const char* c_label = NULL;
7880
if (label != NULL) {
7981
c_label = (*env)->GetStringUTFChars(env, label, NULL);
82+
if (c_label == NULL) {
83+
return EXCEPTION_THROWN;
84+
}
8085
}
8186
if (protocol != NULL) {
8287
init.protocol = (*env)->GetStringUTFChars(env, protocol, NULL);
88+
if (init.protocol == NULL) {
89+
if (c_label != NULL) {
90+
(*env)->ReleaseStringUTFChars(env, label, c_label);
91+
}
92+
return EXCEPTION_THROWN;
93+
}
8394
}
8495

8596
const jint result = rtcCreateDataChannelEx(peerHandle, c_label, &init);
@@ -128,7 +139,14 @@ JNIEXPORT jint JNICALL Java_tel_schich_libdatachannel_LibDataChannelNative_rtcSe
128139
}
129140
char* buffer = (*env)->GetDirectBufferAddress(env, data);
130141
if (buffer == NULL) {
131-
return RTC_ERR_SUCCESS;
142+
return WRAP_ERROR(env, RTC_ERR_INVALID);
143+
}
144+
jlong capacity = (*env)->GetDirectBufferCapacity(env, data);
145+
if (capacity < 0 || offset < 0 || offset > capacity) {
146+
return WRAP_ERROR(env, RTC_ERR_INVALID);
147+
}
148+
if (length >= 0 && ((jlong) offset + (jlong) length > capacity)) {
149+
return WRAP_ERROR(env, RTC_ERR_TOO_SMALL);
132150
}
133151
char* buffer_offset = buffer + offset;
134152
return rtcSendMessage(channelHandle, buffer_offset, length);
@@ -140,31 +158,29 @@ JNIEXPORT jobject JNICALL Java_tel_schich_libdatachannel_LibDataChannelNative_rt
140158
if (size == 0) {
141159
return NULL;
142160
}
143-
void* buffer = malloc(size);
144-
if (buffer == NULL) {
145-
throw_native_exception(env, "Failed to allocate memory for message");
161+
162+
jobject byteBuffer = call_tel_schich_libdatachannel_LibDataChannel_allocate(env, size);
163+
if (byteBuffer == NULL) {
164+
return NULL;
165+
}
166+
void* target = (*env)->GetDirectBufferAddress(env, byteBuffer);
167+
if (target == NULL) {
168+
(*env)->DeleteLocalRef(env, byteBuffer);
169+
throw_native_exception(env, "LibDataChannel allocator must return a direct ByteBuffer");
146170
return NULL;
147171
}
148-
int result = rtcReceiveMessage(channelHandle, buffer, &size);
172+
173+
int receiveSize = size;
174+
int result = rtcReceiveMessage(channelHandle, target, &receiveSize);
149175
if (result == RTC_ERR_NOT_AVAIL) {
150-
free(buffer);
176+
(*env)->DeleteLocalRef(env, byteBuffer);
151177
return NULL;
152178
}
153179
if (result < 0) {
154-
free(buffer);
155180
WRAP_ERROR(env, result);
181+
(*env)->DeleteLocalRef(env, byteBuffer);
156182
return NULL;
157183
}
158-
WRAP_ERROR(env, result);
159-
160-
jobject byteBuffer = (*env)->NewDirectByteBuffer(env, buffer, size);
161-
if (byteBuffer == NULL) {
162-
free(buffer);
163-
throw_native_exception(env, "Failed to create direct byte buffer");
164-
return NULL;
165-
}
166-
// ensure the buffer cleanup is managed
167-
call_tel_schich_libdatachannel_LibDataChannel_freeOnGarbageCollection(env, byteBuffer, (jlong) (intptr_t) buffer);
168184
return byteBuffer;
169185
}
170186

@@ -175,7 +191,11 @@ JNIEXPORT jint JNICALL Java_tel_schich_libdatachannel_LibDataChannelNative_rtcRe
175191
}
176192
char* base = (*env)->GetDirectBufferAddress(env, buffer);
177193
if (base == NULL) {
178-
return 0;
194+
return WRAP_ERROR(env, RTC_ERR_INVALID);
195+
}
196+
jlong bufferCapacity = (*env)->GetDirectBufferCapacity(env, buffer);
197+
if (bufferCapacity < 0 || offset < 0 || capacity < 0 || offset > bufferCapacity || ((jlong) offset + (jlong) capacity > bufferCapacity)) {
198+
return WRAP_ERROR(env, RTC_ERR_INVALID);
179199
}
180200

181201
char* data = base + offset;

‎jni/src/native_peer.c‎

Lines changed: 68 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,31 @@ Java_tel_schich_libdatachannel_LibDataChannelNative_rtcCreatePeerConnection(JNIE
8888

8989
for (int i = 0; i < config.iceServersCount; i++) {
9090
serverStrings[i] = (*env)->GetObjectArrayElement(env, iceServers, i);// we need a reference to release later
91+
if (serverStrings[i] == NULL) {
92+
for (int j = 0; j < i; j++) {
93+
if (config.iceServers[j] != NULL) {
94+
(*env)->ReleaseStringUTFChars(env, serverStrings[j], config.iceServers[j]);
95+
}
96+
(*env)->DeleteLocalRef(env, serverStrings[j]);
97+
}
98+
free(config.iceServers);
99+
free(serverStrings);
100+
throw_native_exception(env, "iceServers must not contain null values");
101+
return EXCEPTION_THROWN;
102+
}
91103
config.iceServers[i] = (*env)->GetStringUTFChars(env, serverStrings[i], NULL);
92104
if (config.iceServers[i] == NULL) {
93105
// release everything and throw an exception
94-
for (int j = 0; j < i; j++) {
95-
(*env)->ReleaseStringUTFChars(env, serverStrings[j], config.iceServers[j]);
106+
for (int j = 0; j <= i; j++) {
107+
if (j < i && config.iceServers[j] != NULL) {
108+
(*env)->ReleaseStringUTFChars(env, serverStrings[j], config.iceServers[j]);
109+
}
110+
if (serverStrings[j] != NULL) {
111+
(*env)->DeleteLocalRef(env, serverStrings[j]);
112+
}
96113
}
97114
free(config.iceServers);
98115
free(serverStrings);
99-
throw_native_exception(env, "Failed to get ice server string!");
100116
return EXCEPTION_THROWN;
101117
}
102118
}
@@ -105,9 +121,34 @@ Java_tel_schich_libdatachannel_LibDataChannelNative_rtcCreatePeerConnection(JNIE
105121

106122
if (proxyServer != NULL) {
107123
config.proxyServer = (*env)->GetStringUTFChars(env, proxyServer, NULL);
124+
if (config.proxyServer == NULL) {
125+
if (config.iceServers != NULL && config.iceServersCount > 0) {
126+
for (int i = 0; i < config.iceServersCount; i++) {
127+
(*env)->ReleaseStringUTFChars(env, serverStrings[i], config.iceServers[i]);
128+
(*env)->DeleteLocalRef(env, serverStrings[i]);
129+
}
130+
free(config.iceServers);
131+
free(serverStrings);
132+
}
133+
return EXCEPTION_THROWN;
134+
}
108135
}
109136
if (bindAddress != NULL) {
110137
config.bindAddress = (*env)->GetStringUTFChars(env, bindAddress, NULL);
138+
if (config.bindAddress == NULL) {
139+
if (proxyServer != NULL) {
140+
(*env)->ReleaseStringUTFChars(env, proxyServer, config.proxyServer);
141+
}
142+
if (config.iceServers != NULL && config.iceServersCount > 0) {
143+
for (int i = 0; i < config.iceServersCount; i++) {
144+
(*env)->ReleaseStringUTFChars(env, serverStrings[i], config.iceServers[i]);
145+
(*env)->DeleteLocalRef(env, serverStrings[i]);
146+
}
147+
free(config.iceServers);
148+
free(serverStrings);
149+
}
150+
return EXCEPTION_THROWN;
151+
}
111152
}
112153

113154
const jint result = (jint) rtcCreatePeerConnection(&config);
@@ -120,10 +161,9 @@ Java_tel_schich_libdatachannel_LibDataChannelNative_rtcCreatePeerConnection(JNIE
120161
}
121162

122163
if (config.iceServers != NULL && config.iceServersCount > 0) {
123-
if (serverStrings != NULL) {
124-
for (int i = 0; i < config.iceServersCount; i++) {
125-
(*env)->ReleaseStringUTFChars(env, serverStrings[i], config.iceServers[i]);
126-
}
164+
for (int i = 0; i < config.iceServersCount; i++) {
165+
(*env)->ReleaseStringUTFChars(env, serverStrings[i], config.iceServers[i]);
166+
(*env)->DeleteLocalRef(env, serverStrings[i]);
127167
}
128168
free(config.iceServers);
129169
free(serverStrings);
@@ -141,15 +181,18 @@ JNIEXPORT jint JNICALL
141181
Java_tel_schich_libdatachannel_LibDataChannelNative_rtcDeletePeerConnection(JNIEnv* env, jclass clazz,
142182
const jint peerHandle) {
143183
struct jvm_callback* callback = rtcGetUserPointer(peerHandle);
144-
if (callback != NULL) {
184+
jint result = rtcDeletePeerConnection(peerHandle);
185+
if ((result == RTC_ERR_SUCCESS || result == RTC_ERR_INVALID) && callback != NULL) {
145186
free_callback(env, callback);
146187
}
147-
148-
return rtcDeletePeerConnection(peerHandle);
188+
return result;
149189
}
150190

151191

152192
JNIEXPORT jint JNICALL Java_tel_schich_libdatachannel_LibDataChannelNative_rtcSetLocalDescription(JNIEnv* env, jclass clazz, const jint peerHandle, const jstring type) {
193+
if (type == NULL) {
194+
return WRAP_ERROR(env, rtcSetLocalDescription(peerHandle, NULL));
195+
}
153196
const char* c_type = (*env)->GetStringUTFChars(env, type, NULL);
154197
if (c_type == NULL) {
155198
THROW_FAILED_GET_STR(env, type);
@@ -169,20 +212,29 @@ JNIEXPORT jstring JNICALL Java_tel_schich_libdatachannel_LibDataChannelNative_rt
169212
}
170213

171214
JNIEXPORT jint JNICALL Java_tel_schich_libdatachannel_LibDataChannelNative_rtcSetRemoteDescription(JNIEnv* env, jclass clazz, const jint peerHandle, const jstring sdp, const jstring type) {
215+
if (sdp == NULL) {
216+
throw_native_exception(env, "sdp must not be null");
217+
return EXCEPTION_THROWN;
218+
}
172219
const char* c_sdp = (*env)->GetStringUTFChars(env, sdp, NULL);
173220
if (c_sdp == NULL) {
174221
THROW_FAILED_GET_STR(env, sdp);
175222
return EXCEPTION_THROWN;
176223
}
177-
const char* c_type = (*env)->GetStringUTFChars(env, type, NULL);
178-
if (c_type == NULL) {
179-
(*env)->ReleaseStringUTFChars(env, sdp, c_sdp);
180-
THROW_FAILED_GET_STR(env, type);
181-
return EXCEPTION_THROWN;
224+
const char* c_type = NULL;
225+
if (type != NULL) {
226+
c_type = (*env)->GetStringUTFChars(env, type, NULL);
227+
if (c_type == NULL) {
228+
(*env)->ReleaseStringUTFChars(env, sdp, c_sdp);
229+
THROW_FAILED_GET_STR(env, type);
230+
return EXCEPTION_THROWN;
231+
}
182232
}
183233
const int result = WRAP_ERROR(env, rtcSetRemoteDescription(peerHandle, c_sdp, c_type));
184234
(*env)->ReleaseStringUTFChars(env, sdp, c_sdp);
185-
(*env)->ReleaseStringUTFChars(env, type, c_type);
235+
if (c_type != NULL) {
236+
(*env)->ReleaseStringUTFChars(env, type, c_type);
237+
}
186238
return result;
187239
}
188240

‎jni/src/native_track.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ JNIEXPORT jint JNICALL Java_tel_schich_libdatachannel_LibDataChannelNative_rtcAd
77
const char* chars = NULL;
88
if (sdp != NULL) {
99
chars = (*env)->GetStringUTFChars(env, sdp, NULL);
10+
if (chars == NULL) {
11+
return EXCEPTION_THROWN;
12+
}
1013
}
1114
const int result = rtcAddTrack(peerHandle, chars);
1215
rtcSetUserPointer(result, rtcGetUserPointer(peerHandle));

0 commit comments

Comments
 (0)