http_aws_sigv4: avoid risk of overflowed constant

- Simplify canon_query() a bit. Avoid unconditionally using length -1
  where length risks being zero at times. Pointed out by Coverity.
- Fix indent errors
- narrow some variable scopes
- fix keywords in tests

Closes #17402
This commit is contained in:
Daniel Stenberg 2025-05-21 08:24:39 +02:00
parent 5b4bd55006
commit 8c8186eadc
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
3 changed files with 61 additions and 68 deletions

View File

@ -72,15 +72,16 @@ struct pair {
static void dyn_array_free(struct dynbuf *db, size_t num_elements);
static void pair_array_free(struct pair *pair_array, size_t num_elements);
static CURLcode split_to_dyn_array(const char *source, char split_by,
struct dynbuf db[MAX_QUERY_COMPONENTS], size_t *num_splits);
static CURLcode split_to_dyn_array(const char *source,
struct dynbuf db[MAX_QUERY_COMPONENTS],
size_t *num_splits);
static bool is_reserved_char(const char c);
static CURLcode uri_encode_path(struct Curl_str *original_path,
struct dynbuf *new_path);
struct dynbuf *new_path);
static CURLcode encode_query_component(char *component, size_t len,
struct dynbuf *db);
struct dynbuf *db);
static CURLcode http_aws_decode_encode(const char *in, size_t in_len,
struct dynbuf *out);
struct dynbuf *out);
static bool should_urlencode(struct Curl_str *service_name);
static void sha256_to_hex(char *dst, unsigned char *sha)
@ -577,21 +578,12 @@ UNITTEST CURLcode canon_query(const char *query, struct dynbuf *dq)
size_t num_query_components;
size_t counted_query_components = 0;
size_t index;
size_t in_key_len;
size_t in_value_len;
size_t query_part_len;
const char *in_key;
char *in_value;
char *offset;
char *key_ptr;
char *value_ptr;
const char *query_part;
if(!query)
return result;
result = split_to_dyn_array(query, '&', &query_array[0],
&num_query_components);
result = split_to_dyn_array(query, &query_array[0],
&num_query_components);
if(result) {
goto fail;
}
@ -599,11 +591,12 @@ UNITTEST CURLcode canon_query(const char *query, struct dynbuf *dq)
/* Create list of pairs, each pair containing an encoded query
* component */
for(index = 0; index < num_query_components;
index++) {
query_part_len = curlx_dyn_len(&query_array[index]);
query_part = curlx_dyn_ptr(&query_array[index]);
for(index = 0; index < num_query_components; index++) {
const char *in_key;
size_t in_key_len;
char *offset;
size_t query_part_len = curlx_dyn_len(&query_array[index]);
char *query_part = curlx_dyn_ptr(&query_array[index]);
in_key = query_part;
@ -622,17 +615,18 @@ UNITTEST CURLcode canon_query(const char *query, struct dynbuf *dq)
/* Decode/encode the key */
result = http_aws_decode_encode(in_key, in_key_len,
&encoded_query_array[index].key);
&encoded_query_array[index].key);
if(result) {
goto fail;
}
/* Decode/encode the value if it exists */
if(offset && offset != (query_part + query_part_len - 1)) {
in_value = offset + 1;
size_t in_value_len;
const char *in_value = offset + 1;
in_value_len = query_part + query_part_len - (offset + 1);
result = http_aws_decode_encode(in_value, in_value_len,
&encoded_query_array[index].value);
&encoded_query_array[index].value);
if(result) {
goto fail;
}
@ -650,30 +644,33 @@ UNITTEST CURLcode canon_query(const char *query, struct dynbuf *dq)
/* Sort the encoded query components by key and value */
qsort(&encoded_query_array, num_query_components,
sizeof(struct pair), compare_func);
sizeof(struct pair), compare_func);
/* Append the query components together to make a full query string */
for(index = 0; index < num_query_components; index++) {
key_ptr = curlx_dyn_ptr(&encoded_query_array[index].key);
value_ptr = curlx_dyn_ptr(&encoded_query_array[index].value);
if(value_ptr && strlen(value_ptr)) {
result = curlx_dyn_addf(dq, "%s=%s&", key_ptr, value_ptr);
}
else {
/* Empty value is always encoded to key= */
result = curlx_dyn_addf(dq, "%s=&", key_ptr);
}
if(result) {
goto fail;
if(index)
result = curlx_dyn_addn(dq, "&", 1);
if(!result) {
char *key_ptr = curlx_dyn_ptr(&encoded_query_array[index].key);
char *value_ptr = curlx_dyn_ptr(&encoded_query_array[index].value);
size_t vlen = curlx_dyn_len(&encoded_query_array[index].value);
if(value_ptr && vlen) {
result = curlx_dyn_addf(dq, "%s=%s", key_ptr, value_ptr);
}
else {
/* Empty value is always encoded to key= */
result = curlx_dyn_addf(dq, "%s=", key_ptr);
}
}
if(result)
break;
}
/* Remove trailing & */
result = curlx_dyn_setlen(dq, curlx_dyn_len(dq)-1);
fail:
pair_array_free(&encoded_query_array[0], counted_query_components);
if(counted_query_components)
/* the encoded_query_array might not be initialized yet */
pair_array_free(&encoded_query_array[0], counted_query_components);
dyn_array_free(&query_array[0], num_query_components);
return result;
}
@ -1010,41 +1007,38 @@ static void dyn_array_free(struct dynbuf *db, size_t num_elements)
{
size_t index;
for(index = 0; index < num_elements; index++) {
for(index = 0; index < num_elements; index++)
curlx_dyn_free((&db[index]));
}
}
/*
* Splits source string by split_by, and creates an array of dynbuf in db
* db is initialized by this function
* Splits source string by SPLIT_BY, and creates an array of dynbuf in db.
* db is initialized by this function.
* Caller is responsible for freeing the array elements with dyn_array_free
*/
static CURLcode split_to_dyn_array(const char *source, char split_by,
struct dynbuf db[MAX_QUERY_COMPONENTS], size_t *num_splits_out)
#define SPLIT_BY '&'
static CURLcode split_to_dyn_array(const char *source,
struct dynbuf db[MAX_QUERY_COMPONENTS],
size_t *num_splits_out)
{
CURLcode result = CURLE_OK;
size_t len = strlen(source);
size_t pos = 0; /* Position in result buffer */
size_t start = 0; /* Start of current segment */
size_t segment_length = 0;
size_t index = 0;
size_t num_splits;
size_t num_splits = 0;
/* Split source_ptr on split_by and store the segment offsets and
* length in array */
num_splits = 0;
/* Split source_ptr on SPLIT_BY and store the segment offsets and length in
* array */
for(pos = 0; pos < len; pos++) {
if(source[pos] == split_by) {
if(source[pos] == SPLIT_BY) {
if(segment_length) {
curlx_dyn_init(&db[index], segment_length + 1);
result = curlx_dyn_addn(&db[index], &source[start],
segment_length);
segment_length);
if(result) {
goto fail;
}
@ -1064,14 +1058,14 @@ static CURLcode split_to_dyn_array(const char *source, char split_by,
if(segment_length) {
curlx_dyn_init(&db[index], segment_length + 1);
result = curlx_dyn_addn(&db[index], &source[start],
segment_length);
segment_length);
if(result) {
goto fail;
}
if(++num_splits == MAX_QUERY_COMPONENTS) {
goto fail;
}
}
}
fail:
*num_splits_out = num_splits;
return result;
@ -1084,9 +1078,8 @@ static bool is_reserved_char(const char c)
}
static CURLcode uri_encode_path(struct Curl_str *original_path,
struct dynbuf *new_path)
struct dynbuf *new_path)
{
const char *p = curlx_str(original_path);
CURLcode result = CURLE_OK;
size_t index;
@ -1113,9 +1106,8 @@ fail:
static CURLcode encode_query_component(char *component, size_t len,
struct dynbuf *db)
struct dynbuf *db)
{
size_t index;
CURLcode result = CURLE_OK;
unsigned char this_char;
@ -1149,17 +1141,16 @@ fail:
*/
static CURLcode http_aws_decode_encode(const char *in, size_t in_len,
struct dynbuf *out)
struct dynbuf *out)
{
CURLcode result = CURLE_OK;
char *out_s;
size_t out_s_len;
CURLcode result =
Curl_urldecode(in, in_len, &out_s, &out_s_len, REJECT_NADA);
result = Curl_urldecode(in, in_len, &out_s, &out_s_len, REJECT_NADA);
if(result) {
if(result)
goto fail;
}
result = encode_query_component(out_s, out_s_len, out);
Curl_safefree(out_s);
fail:

View File

@ -3,6 +3,7 @@
<keywords>
unittest
canon_string
aws-sigv4
</keywords>
</info>

View File

@ -3,6 +3,7 @@
<keywords>
unittest
canon_query
aws-sigv4
</keywords>
</info>