Skip to content

Commit

Permalink
[2156] Replace variable length arrays with alloca()
Browse files Browse the repository at this point in the history
  • Loading branch information
SwooshyCueb committed Apr 27, 2024
1 parent de04e4d commit 5d62434
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 26 deletions.
14 changes: 14 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ if (NOT CMAKE_CONFIGURATION_TYPES AND NOT CMAKE_BUILD_TYPE)
message(STATUS "Setting unspecified CMAKE_BUILD_TYPE to '${CMAKE_BUILD_TYPE}'")
endif()

include(CheckCCompilerFlag)
include(CheckCXXCompilerFlag)

if (CMAKE_CXX_COMPILER_ID MATCHES ".*Clang")
set(IRODS_BUILD_WITH_WERROR_DEFAULT ON)
else()
Expand All @@ -47,6 +50,17 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-fpermissive>)
endif()

# Variable length arrays can crash clang (#2156), so let's turn on warnings for them.
# -Wvla is not enabled by -Wall or -Wextra, so we have to do it manually.
check_c_compiler_flag(-Wvla can_use_wvla_c)
check_cxx_compiler_flag(-Wvla can_use_wvla_cxx)
if (can_use_wvla_c)
add_compile_options($<$<COMPILE_LANGUAGE:C>:-Wvla>)
endif()
if (can_use_wvla_cxx)
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wvla>)
endif()

if (NOT DEFINED THREADS_PREFER_PTHREAD_FLAG)
set(THREADS_PREFER_PTHREAD_FLAG TRUE)
endif()
Expand Down
5 changes: 3 additions & 2 deletions libs3/src/bucket.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
*
************************************************************************** **/

#include <alloca.h>
#include <string.h>
#include <stdlib.h>
#include "libs3/libs3.h"
Expand Down Expand Up @@ -464,7 +465,7 @@ static S3Status make_list_bucket_callback(ListBucketData* lbData)
int isTruncated = (!strcmp(lbData->isTruncated, "true") || !strcmp(lbData->isTruncated, "1")) ? 1 : 0;

// Convert the contents
S3ListBucketContent contents[lbData->contentsCount];
S3ListBucketContent* contents = alloca(sizeof(S3ListBucketContent) * lbData->contentsCount);

int contentsCount = lbData->contentsCount;
for (i = 0; i < contentsCount; i++) {
Expand All @@ -480,7 +481,7 @@ static S3Status make_list_bucket_callback(ListBucketData* lbData)

// Make the common prefixes array
int commonPrefixesCount = lbData->commonPrefixesCount;
char* commonPrefixes[commonPrefixesCount];
char** commonPrefixes = alloca(sizeof(char*) * commonPrefixesCount);
for (i = 0; i < commonPrefixesCount; i++) {
commonPrefixes[i] = lbData->commonPrefixes[i];
}
Expand Down
7 changes: 4 additions & 3 deletions libs3/src/multipart.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
*
************************************************************************** **/

#include <alloca.h>
#include <string.h>
#include <stdlib.h>
#include "libs3/libs3.h"
Expand Down Expand Up @@ -506,7 +507,7 @@ static S3Status make_list_multipart_callback(ListMultipartData* lmData)
int isTruncated = (!strcmp(lmData->isTruncated, "true") || !strcmp(lmData->isTruncated, "1")) ? 1 : 0;

// Convert the contents
S3ListMultipartUpload uploads[lmData->uploadsCount];
S3ListMultipartUpload* uploads = alloca(sizeof(S3ListMultipartUpload) * lmData->uploadsCount);

int uploadsCount = lmData->uploadsCount;
for (i = 0; i < uploadsCount; i++) {
Expand All @@ -524,7 +525,7 @@ static S3Status make_list_multipart_callback(ListMultipartData* lmData)

// Make the common prefixes array
int commonPrefixesCount = lmData->commonPrefixesCount;
char* commonPrefixes[commonPrefixesCount];
char** commonPrefixes = alloca(sizeof(char*) * commonPrefixesCount);
for (i = 0; i < commonPrefixesCount; i++) {
commonPrefixes[i] = lmData->commonPrefixes[i];
}
Expand All @@ -547,7 +548,7 @@ static S3Status make_list_parts_callback(ListPartsData* lpData)
int isTruncated = (!strcmp(lpData->isTruncated, "true") || !strcmp(lpData->isTruncated, "1")) ? 1 : 0;

// Convert the contents
S3ListPart Parts[lpData->partsCount];
S3ListPart* Parts = alloca(sizeof(S3ListPart) * lpData->partsCount);
int partsCount = lpData->partsCount;
for (i = 0; i < partsCount; i++) {
S3ListPart* partDest = &(Parts[i]);
Expand Down
51 changes: 30 additions & 21 deletions libs3/src/request.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
*
************************************************************************** **/

#include <alloca.h>
#include <ctype.h>
#include <pthread.h>
#include <stdlib.h>
Expand Down Expand Up @@ -821,7 +822,7 @@ static void sort_query_string(const char* queryString, char* result, unsigned in
tmp++;
}

const char* params[numParams];
const char** params = alloca(sizeof(const char*) * numParams);

// Where did strdup go?!??
int queryStringLen = strlen(queryString);
Expand Down Expand Up @@ -875,9 +876,10 @@ static void canonicalize_query_string(const char* queryParams,
#define append(str) len += snprintf(&(buffer[len]), buffer_size - len, "%s", str)

if (queryParams && queryParams[0]) {
char sorted[strlen(queryParams) * 2];
const size_t sortedSize = sizeof(char) * strlen(queryParams) * 2;
char* sorted = alloca(sortedSize);
sorted[0] = '\0';
sort_query_string(queryParams, sorted, sizeof(sorted));
sort_query_string(queryParams, sorted, sortedSize);
append(sorted);
}

Expand Down Expand Up @@ -942,26 +944,32 @@ static const char* http_request_type_to_verb(HttpRequestType requestType)
static S3Status compose_auth_header(const RequestParams* params, RequestComputedValues* values)
{
const char* httpMethod = http_request_type_to_verb(params->httpRequestType);
int canonicalRequestLen = strlen(httpMethod) + 1 + strlen(values->canonicalURI) + 1 +
strlen(values->canonicalQueryString) + 1 + strlen(values->canonicalizedSignatureHeaders) +
1 + strlen(values->signedHeaders) + 1 + 2 * S3_SHA256_DIGEST_LENGTH +
1; // 2 hex digits for each byte

// clang-format off
int canonicalRequestLen =
(
strlen(httpMethod) + 1 +
strlen(values->canonicalURI) + 1 +
strlen(values->canonicalQueryString) + 1 +
strlen(values->canonicalizedSignatureHeaders) + 1 +
strlen(values->signedHeaders) + 1 +
2 * S3_SHA256_DIGEST_LENGTH + 1 // 2 hex digits for each byte
) * sizeof(char);
// clang-format on
int len = 0;

char canonicalRequest[canonicalRequestLen];
char* canonicalRequest = alloca(canonicalRequestLen);

#define buf_append(buf, format, ...) len += snprintf(&(buf[len]), sizeof(buf) - len, format, __VA_ARGS__)
#define buf_append(buf, bufLen, format, ...) len += snprintf(&(buf[len]), bufLen - len, format, __VA_ARGS__)

canonicalRequest[0] = '\0';
buf_append(canonicalRequest, "%s\n", httpMethod);
buf_append(canonicalRequest, "%s\n", values->canonicalURI);
buf_append(canonicalRequest, "%s\n", values->canonicalQueryString);
buf_append(canonicalRequest, "%s\n", values->canonicalizedSignatureHeaders);
buf_append(canonicalRequest, "%s\n", values->signedHeaders);
buf_append(canonicalRequest, canonicalRequestLen, "%s\n", httpMethod);
buf_append(canonicalRequest, canonicalRequestLen, "%s\n", values->canonicalURI);
buf_append(canonicalRequest, canonicalRequestLen, "%s\n", values->canonicalQueryString);
buf_append(canonicalRequest, canonicalRequestLen, "%s\n", values->canonicalizedSignatureHeaders);
buf_append(canonicalRequest, canonicalRequestLen, "%s\n", values->signedHeaders);

buf_append(canonicalRequest, "%s", values->payloadHash);
//buf_append(canonicalRequest, "%s", values->dateHeader);
buf_append(canonicalRequest, canonicalRequestLen, "%s", values->payloadHash);
//buf_append(canonicalRequest, canonicalRequestLen, "%s", values->dateHeader);

#ifdef SIGNATURE_DEBUG
printf("--\nCanonical Request:\n%s\n", canonicalRequest);
Expand All @@ -979,7 +987,7 @@ static S3Status compose_auth_header(const RequestParams* params, RequestComputed
canonicalRequestHashHex[0] = '\0';
int i = 0;
for (; i < S3_SHA256_DIGEST_LENGTH; i++) {
buf_append(canonicalRequestHashHex, "%02x", canonicalRequestHash[i]);
buf_append(canonicalRequestHashHex, sizeof(canonicalRequestHashHex), "%02x", canonicalRequestHash[i]);
}

const char* awsRegion = S3_DEFAULT_REGION;
Expand All @@ -1003,8 +1011,9 @@ static S3Status compose_auth_header(const RequestParams* params, RequestComputed
#endif

const char* secretAccessKey = params->bucketContext.secretAccessKey;
char accessKey[strlen(secretAccessKey) + 5];
snprintf(accessKey, sizeof(accessKey), "AWS4%s", secretAccessKey);
const size_t accessKeySize = sizeof(char) * (strlen(secretAccessKey) + 5);
char* accessKey = alloca(accessKeySize);
snprintf(accessKey, accessKeySize, "AWS4%s", secretAccessKey);

#ifdef __APPLE__
unsigned char dateKey[S3_SHA256_DIGEST_LENGTH];
Expand Down Expand Up @@ -1059,7 +1068,7 @@ static S3Status compose_auth_header(const RequestParams* params, RequestComputed
len = 0;
values->requestSignatureHex[0] = '\0';
for (i = 0; i < S3_SHA256_DIGEST_LENGTH; i++) {
buf_append(values->requestSignatureHex, "%02x", finalSignature[i]);
buf_append(values->requestSignatureHex, sizeof(values->requestSignatureHex), "%02x", finalSignature[i]);
}

snprintf(values->authCredential,
Expand Down

0 comments on commit 5d62434

Please sign in to comment.