Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn integers downcast 64to32 6186 v1 #9105

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1599,7 +1599,7 @@ jobs:
CC: "clang-14"
CXX: "clang++-14"
RUSTFLAGS: "-C instrument-coverage"
CFLAGS: "-fprofile-instr-generate -fcoverage-mapping -O0 -g -fno-strict-aliasing -fsanitize=address -fno-omit-frame-pointer -fPIC -Wno-unused-parameter -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION=1 -Wimplicit-int-float-conversion -Wimplicit-int-conversion -Werror"
CFLAGS: "-fprofile-instr-generate -fcoverage-mapping -O0 -g -fno-strict-aliasing -fsanitize=address -fno-omit-frame-pointer -fPIC -Wno-unused-parameter -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION=1 -Wimplicit-int-float-conversion -Wimplicit-int-conversion -Wshorten-64-to-32 -Werror"
CXXFLAGS: "-fprofile-instr-generate -fcoverage-mapping -O0 -g -fno-strict-aliasing -fsanitize=address -fno-omit-frame-pointer -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION=1 -stdlib=libc++ -Wimplicit-int-float-conversion -Wimplicit-int-conversion"
ac_cv_func_malloc_0_nonnull: "yes"
ac_cv_func_realloc_0_nonnull: "yes"
Expand Down
2 changes: 1 addition & 1 deletion src/app-layer-detect-proto.c
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ static uint32_t AppLayerProtoDetectProbingParserGetMask(AppProto alproto)
FatalError("Unknown protocol detected - %u", alproto);
}

SCReturnUInt(1UL << (uint32_t)alproto);
SCReturnUInt(1 << (uint32_t)alproto);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should AppLayerProtoDetectProbingParserGetMask return a u64 ?
But then flow structure needs to grow 2 u32 into 2 u64...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why an u64?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we will soon have more than 32 app-layer protocols, right ?

}

static AppLayerProtoDetectProbingParserElement *AppLayerProtoDetectProbingParserElementAlloc(void)
Expand Down
3 changes: 1 addition & 2 deletions src/app-layer-dnp3-objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ static int DNP3ReadUint24(const uint8_t **buf, uint32_t *len, uint32_t *out)
*out = ((uint32_t)(*buf)[0] << 16) | ((uint32_t)(*buf)[1] << 8) |
(uint32_t)(*buf)[2];
#elif __BYTE_ORDER == __LITTLE_ENDIAN
*out = ((uint64_t)(*buf)[0]) | ((uint64_t)(*buf)[1] << 8) |
((uint64_t)(*buf)[2] << 16);
*out = ((uint32_t)(*buf)[0]) | ((uint32_t)(*buf)[1] << 8) | ((uint32_t)(*buf)[2] << 16);
#endif

*buf += 3;
Expand Down
2 changes: 1 addition & 1 deletion src/app-layer-frames.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ typedef struct Frame {
typedef struct Frames {
uint16_t cnt;
uint16_t dyn_size; /**< size in elements of `dframes` */
uint32_t left_edge_rel;
uint64_t left_edge_rel;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@victorjulien is this correct for frames ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no it's u32 intentionally, it's relative to something, I think the stream base seq. So when used it's added to that other thing.

uint64_t base_id;
Frame sframes[FRAMES_STATIC_CNT]; /**< static frames */
Frame *dframes; /**< dynamically allocated space for more frames */
Expand Down
2 changes: 1 addition & 1 deletion src/app-layer-ftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ static AppLayerResult FTPGetLineForDirection(
// lf_idx = 5010
// max_line_len = 4096
uint32_t o_consumed = input->consumed;
input->consumed = lf_idx - input->buf + 1;
input->consumed = (uint32_t)(lf_idx - input->buf + 1);
line->len = input->consumed - o_consumed;
input->len -= line->len;
line->lf_found = true;
Expand Down
4 changes: 2 additions & 2 deletions src/app-layer-htp-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ int HTPFileOpen(HtpState *s, HtpTxUserData *tx, const uint8_t *filename, uint16_
*/
int HTPParseContentRange(bstr *rawvalue, HTTPContentRange *range)
{
uint32_t len = bstr_len(rawvalue);
uint32_t len = (uint32_t)bstr_len(rawvalue);
return rs_http_parse_content_range(range, bstr_ptr(rawvalue), len);
}

Expand Down Expand Up @@ -175,7 +175,7 @@ int HTPFileOpenWithRange(HtpState *s, HtpTxUserData *txud, const uint8_t *filena
uint8_t *keyurl;
uint32_t keylen;
if (tx->request_hostname != NULL) {
keylen = bstr_len(tx->request_hostname) + filename_len;
keylen = (uint32_t)bstr_len(tx->request_hostname) + filename_len;
keyurl = SCMalloc(keylen);
if (keyurl == NULL) {
SCReturnInt(-1);
Expand Down
35 changes: 27 additions & 8 deletions src/app-layer-htp-range.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ static inline bool ContainerValueRangeTimeout(HttpRangeContainerFile *cu, const
return false;
}

static void ContainerUrlRangeUpdate(HttpRangeContainerFile *cu, uint32_t expire)
static void ContainerUrlRangeUpdate(HttpRangeContainerFile *cu, uint64_t expire)
{
cu->expire = expire;
}
Expand Down Expand Up @@ -410,9 +410,9 @@ int HttpRangeAppendData(const StreamingBufferConfig *sbcfg, HttpRangeContainerBl
if (c->files) {
if (data == NULL) {
// gap overlapping already known data
r = FileAppendData(c->files, sbcfg, NULL, len - c->toskip);
r = FileAppendData(c->files, sbcfg, NULL, (uint32_t)(len - c->toskip));
} else {
r = FileAppendData(c->files, sbcfg, data + c->toskip, len - c->toskip);
r = FileAppendData(c->files, sbcfg, data + c->toskip, (uint32_t)(len - c->toskip));
}
}
c->toskip = 0;
Expand Down Expand Up @@ -543,14 +543,24 @@ File *HttpRangeClose(const StreamingBufferConfig *sbcfg, HttpRangeContainerBlock
// a new range just begins where we ended, append it
if (range->gap > 0) {
// if the range had a gap, begin by it
if (FileAppendData(c->container->files, sbcfg, NULL, range->gap) != 0) {
uint32_t gap = (uint32_t)range->gap;
if (range->gap > UINT32_MAX) {
gap = UINT32_MAX;
}
if (FileAppendData(c->container->files, sbcfg, NULL, gap) != 0) {
c->container->lastsize = f->size;
HttpRangeFileClose(sbcfg, c->container, flags | FILE_TRUNCATED);
c->container->error = true;
return f;
}
}
if (FileAppendData(c->container->files, sbcfg, range->buffer, range->offset) != 0) {
if (range->offset > UINT32_MAX) {
c->container->lastsize = f->size;
HttpRangeFileClose(sbcfg, c->container, flags | FILE_TRUNCATED);
c->container->error = true;
return f;
} else if (FileAppendData(c->container->files, sbcfg, range->buffer,
(uint32_t)range->offset) != 0) {
c->container->lastsize = f->size;
HttpRangeFileClose(sbcfg, c->container, flags | FILE_TRUNCATED);
c->container->error = true;
Expand All @@ -562,7 +572,11 @@ File *HttpRangeClose(const StreamingBufferConfig *sbcfg, HttpRangeContainerBlock
if (overlap < range->offset) {
if (range->gap > 0) {
// if the range had a gap, begin by it
if (FileAppendData(c->container->files, sbcfg, NULL, range->gap) != 0) {
uint32_t gap = (uint32_t)range->gap;
if (range->gap > UINT32_MAX) {
gap = UINT32_MAX;
}
if (FileAppendData(c->container->files, sbcfg, NULL, gap) != 0) {
c->container->lastsize = f->size;
HttpRangeFileClose(sbcfg, c->container, flags | FILE_TRUNCATED);
c->container->error = true;
Expand All @@ -571,8 +585,13 @@ File *HttpRangeClose(const StreamingBufferConfig *sbcfg, HttpRangeContainerBlock
}
// And the range ends beyond where we ended
// in this case of overlap, only add the extra data
if (FileAppendData(c->container->files, sbcfg, range->buffer + overlap,
range->offset - overlap) != 0) {
if (range->offset - overlap > UINT32_MAX) {
c->container->lastsize = f->size;
HttpRangeFileClose(sbcfg, c->container, flags | FILE_TRUNCATED);
c->container->error = true;
return f;
} else if (FileAppendData(c->container->files, sbcfg, range->buffer + overlap,
(uint32_t)(range->offset - overlap)) != 0) {
c->container->lastsize = f->size;
HttpRangeFileClose(sbcfg, c->container, flags | FILE_TRUNCATED);
c->container->error = true;
Expand Down
2 changes: 1 addition & 1 deletion src/app-layer-htp-range.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ typedef struct HttpRangeContainerFile {
/** key length */
uint32_t len;
/** expire time in epoch */
uint32_t expire;
uint64_t expire;
/** pointer to hashtable data, for locking and use count */
THashData *hdata;
/** total expected size of the file in ranges */
Expand Down
32 changes: 17 additions & 15 deletions src/app-layer-htp.c
Original file line number Diff line number Diff line change
Expand Up @@ -560,12 +560,12 @@ static uint32_t AppLayerHtpComputeChunkLength(uint64_t content_len_so_far, uint3
(content_len_so_far < (uint64_t)body_limit) &&
(content_len_so_far + (uint64_t)data_len) > body_limit)
{
chunk_len = body_limit - content_len_so_far;
chunk_len = (uint32_t)(body_limit - content_len_so_far);
} else if ((flags & HTP_STREAM_DEPTH_SET) && stream_depth > 0 &&
(content_len_so_far < (uint64_t)stream_depth) &&
(content_len_so_far + (uint64_t)data_len) > stream_depth)
{
chunk_len = stream_depth - content_len_so_far;
chunk_len = (uint32_t)(stream_depth - content_len_so_far);
}
SCLogDebug("len %u", chunk_len);
return (chunk_len == 0 ? data_len : chunk_len);
Expand Down Expand Up @@ -963,7 +963,7 @@ static AppLayerResult HTPHandleResponseData(Flow *f, void *htp_state, AppLayerPa

htp_time_t ts = { SCTIME_SECS(f->startts), SCTIME_USECS(f->startts) };
htp_tx_t *tx = NULL;
size_t consumed = 0;
uint32_t consumed = 0;
if (input_len > 0) {
const int r = htp_connp_res_data(hstate->connp, &ts, input, input_len);
switch (r) {
Expand All @@ -986,7 +986,7 @@ static AppLayerResult HTPHandleResponseData(Flow *f, void *htp_state, AppLayerPa
if (tx->request_port_number != -1) {
dp = (uint16_t)tx->request_port_number;
}
consumed = htp_connp_res_data_consumed(hstate->connp);
consumed = (uint32_t)htp_connp_res_data_consumed(hstate->connp);
hstate->slice = NULL;
if (!AppLayerRequestProtocolChange(hstate->f, dp, ALPROTO_HTTP2)) {
HTPSetEvent(hstate, NULL, STREAM_TOCLIENT,
Expand Down Expand Up @@ -1273,7 +1273,7 @@ static void HtpRequestBodyMultipartParseHeader(HtpState *hstate,
if (next_line == NULL) {
line_len = header_len;
} else {
line_len = next_line - header;
line_len = (uint32_t)(next_line - header);
}
uint8_t *sc = (uint8_t *)memchr(line, ':', line_len);
if (sc == NULL) {
Expand Down Expand Up @@ -1392,10 +1392,12 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
/* end marker belonging to header_start */
const uint8_t *header_end = NULL;
if (header_start != NULL) {
header_end = Bs2bmSearch(header_start, chunks_buffer_len - (header_start - chunks_buffer),
header_end = Bs2bmSearch(header_start,
(uint32_t)(chunks_buffer_len - (header_start - chunks_buffer)),
(uint8_t *)"\r\n\r\n", 4);
form_end = Bs2bmSearch(header_start, chunks_buffer_len - (header_start - chunks_buffer),
boundary, expected_boundary_end_len);
form_end = Bs2bmSearch(header_start,
(uint32_t)(chunks_buffer_len - (header_start - chunks_buffer)), boundary,
expected_boundary_end_len);
}

SCLogDebug("header_start %p, header_end %p, form_end %p", header_start,
Expand All @@ -1421,7 +1423,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
} else if (header_start > filedata + 2) {
SCLogDebug("some data from last file before the boundary");
/* some data from last file before the boundary */
filedata_len = header_start - filedata - 2;
filedata_len = (uint32_t)(header_start - filedata - 2);
}
}
/* body parsing done, we did not get our form end. Use all data
Expand Down Expand Up @@ -1504,7 +1506,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
uint8_t *filetype = NULL;
uint16_t filetype_len = 0;

uint32_t header_len = header_end - header_start;
uint32_t header_len = (uint32_t)(header_end - header_start);
SCLogDebug("header_len %u", header_len);
uint8_t *header = (uint8_t *)header_start;

Expand Down Expand Up @@ -1546,7 +1548,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
goto end;
}

filedata_len = form_end - (header_end + 4 + 2);
filedata_len = (uint32_t)(form_end - (header_end + 4 + 2));
SCLogDebug("filedata_len %"PRIuMAX, (uintmax_t)filedata_len);

/* or is it? */
Expand Down Expand Up @@ -1587,7 +1589,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
SCLogDebug("chunk doesn't contain form end");

filedata = header_end + 4;
filedata_len = chunks_buffer_len - (filedata - chunks_buffer);
filedata_len = (uint32_t)(chunks_buffer_len - (filedata - chunks_buffer));
SCLogDebug("filedata_len %u (chunks_buffer_len %u)", filedata_len, chunks_buffer_len);

if (filedata_len > chunks_buffer_len) {
Expand All @@ -1609,7 +1611,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
if (header_next == NULL) {
SCLogDebug("more file data to come");

uint32_t offset = (header_end + 4) - chunks_buffer;
uint32_t offset = (uint32_t)((header_end + 4) - chunks_buffer);
SCLogDebug("offset %u", offset);
htud->request_body.body_parsed += offset;

Expand Down Expand Up @@ -1642,7 +1644,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
SCLogDebug("htud->request_body.body_parsed %"PRIu64, htud->request_body.body_parsed);

} else if (header_next - filedata > 2) {
filedata_len = header_next - filedata - 2;
filedata_len = (uint32_t)(header_next - filedata - 2);
SCLogDebug("filedata_len %u", filedata_len);

result = HTPFileOpen(hstate, htud, filename, filename_len, filedata,
Expand All @@ -1668,7 +1670,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
header_start, header_end, form_end);

/* Search next boundary entry after the start of body */
uint32_t cursizeread = header_end - chunks_buffer;
uint32_t cursizeread = (uint32_t)(header_end - chunks_buffer);
header_start = Bs2bmSearch(header_end + 4,
chunks_buffer_len - (cursizeread + 4),
boundary, expected_boundary_len);
Expand Down
10 changes: 6 additions & 4 deletions src/app-layer-http2.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,18 @@ void HTTP2MimicHttp1Request(void *alstate_orig, void *h2s)
return;
}
// else
rs_http2_tx_set_method(h2s, bstr_ptr(h1tx->request_method), bstr_len(h1tx->request_method));
rs_http2_tx_set_method(
h2s, bstr_ptr(h1tx->request_method), (uint32_t)bstr_len(h1tx->request_method));
if (h1tx->request_uri != NULL) {
// A request line without spaces gets interpreted as a request_method
// and has request_uri=NULL
rs_http2_tx_set_uri(h2s, bstr_ptr(h1tx->request_uri), bstr_len(h1tx->request_uri));
rs_http2_tx_set_uri(
h2s, bstr_ptr(h1tx->request_uri), (uint32_t)bstr_len(h1tx->request_uri));
}
size_t nbheaders = htp_table_size(h1tx->request_headers);
for (size_t i = 0; i < nbheaders; i++) {
htp_header_t *h = htp_table_get_index(h1tx->request_headers, i, NULL);
rs_http2_tx_add_header(
h2s, bstr_ptr(h->name), bstr_len(h->name), bstr_ptr(h->value), bstr_len(h->value));
rs_http2_tx_add_header(h2s, bstr_ptr(h->name), (uint32_t)bstr_len(h->name),
bstr_ptr(h->value), (uint32_t)bstr_len(h->value));
}
}