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.

Sign up for GitHub

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

Integer overflow 4527 v1 #6674

Closed
wants to merge 5 commits into from
Closed

Integer overflow 4527 v1 #6674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b90d20
ftp: fix int warnings
catenacyber Dec 6, 2021
9604c72
http: : fix int warnings
catenacyber Dec 6, 2021
05335e4
app: fix int warnings in generic app files
catenacyber Dec 6, 2021
3961d54
smtp: fix int warnings
catenacyber Dec 6, 2021
5e3f3c0
ssl: fix int warnings
catenacyber Dec 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/app-layer-ftp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,14 +642,18 @@ static AppLayerResult FTPParseRequest(Flow *f, void *ftp_state,
* Min size has been checked in FTPParseRequestCommand
* PATH_MAX includes the null
*/
int file_name_len = MIN(PATH_MAX - 1, state->current_line_len - 5);
uint32_t file_name_len = MIN(PATH_MAX - 1, state->current_line_len - 5);
if (file_name_len > UINT16_MAX) {
// truncate the file name if too long for util-file.h
file_name_len = UINT16_MAX;
}
data->file_name = FTPCalloc(file_name_len + 1, sizeof(char));
if (data->file_name == NULL) {
FtpTransferCmdFree(data);
SCReturnStruct(APP_LAYER_ERROR);
}
data->file_name[file_name_len] = 0;
data->file_len = file_name_len;
data->file_len = (uint16_t)file_name_len;
memcpy(data->file_name, state->current_line + 5, file_name_len);
data->cmd = state->command;
data->flow_id = FlowGetId(f);
Expand Down Expand Up @@ -1029,9 +1033,8 @@ static StreamingBufferConfig sbcfg = STREAMING_BUFFER_CONFIG_INITIALIZER;
* \retval 1 when the command is parsed, 0 otherwise
*/
static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state,
AppLayerParserState *pstate,
const uint8_t *input, uint32_t input_len,
void *local_data, int direction)
AppLayerParserState *pstate, const uint8_t *input, uint32_t input_len, void *local_data,
uint8_t direction)
{
uint16_t flags = FileFlowToFlags(f, direction);
int ret = 0;
Expand Down Expand Up @@ -1361,7 +1364,7 @@ uint16_t JsonGetNextLineFromBuffer(const char *buffer, const uint16_t len)
}

char *c = strchr(buffer, '\n');
return c == NULL ? len : c - buffer + 1;
return c == NULL ? len : (uint16_t)(c - buffer + 1);
}

void EveFTPDataAddMetadata(const Flow *f, JsonBuilder *jb)
Expand Down
2 changes: 1 addition & 1 deletion src/app-layer-ftp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ typedef struct FtpLineState_ {

typedef struct FTPString_ {
uint8_t *str;
uint16_t len;
uint32_t len;
TAILQ_ENTRY(FTPString_) next;
} FTPString;

Expand Down
3 changes: 1 addition & 2 deletions src/app-layer-htp-file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,7 @@ int HTPFileOpenWithRange(HtpState *s, HtpTxUserData *txud, const uint8_t *filena
HTTPContentRange crparsed;
if (HTPParseAndCheckContentRange(rawvalue, &crparsed, s, htud) != 0) {
// range is invalid, fall back to classic open
return HTPFileOpen(
s, txud, filename, (uint32_t)filename_len, data, data_len, txid, STREAM_TOCLIENT);
return HTPFileOpen(s, txud, filename, filename_len, data, data_len, txid, STREAM_TOCLIENT);
}
flags = FileFlowToFlags(s->f, STREAM_TOCLIENT);
if ((s->flags & HTP_FLAG_STORE_FILES_TS) ||
Expand Down
3 changes: 2 additions & 1 deletion src/app-layer-htp-range.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ void HttpRangeContainersInit(void)
}
}
if (ConfGetValue("app-layer.protocols.http.byterange.timeout", &str) == 1) {
if (StringParseUint32(&timeout, 10, strlen(str), str) <= 0) {
size_t slen = strlen(str);
if (slen > UINT16_MAX || StringParseUint32(&timeout, 10, (uint16_t)slen, str) <= 0) {
SCLogWarning(SC_ERR_INVALID_VALUE,
"timeout value cannot be deduced: %s,"
" resetting to default",
Expand Down
41 changes: 25 additions & 16 deletions src/app-layer-htp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ static uint32_t AppLayerHtpComputeChunkLength(uint64_t content_len_so_far, uint3
/* below error messages updated up to libhtp 0.5.7 (git 379632278b38b9a792183694a4febb9e0dbd1e7a) */
struct {
const char *msg;
int de;
uint8_t de;
} htp_errors[] = {
{ "GZip decompressor: inflateInit2 failed", HTTP_DECODER_EVENT_GZIP_DECOMPRESSION_FAILED},
{ "Request field invalid: colon missing", HTTP_DECODER_EVENT_REQUEST_FIELD_MISSING_COLON},
Expand All @@ -547,7 +547,7 @@ struct {

struct {
const char *msg;
int de;
uint8_t de;
} htp_warnings[] = {
{ "GZip decompressor:", HTTP_DECODER_EVENT_GZIP_DECOMPRESSION_FAILED},
{ "Request field invalid", HTTP_DECODER_EVENT_REQUEST_HEADER_INVALID},
Expand Down Expand Up @@ -594,7 +594,7 @@ struct {
*
* \retval id the id or 0 in case of not found
*/
static int HTPHandleWarningGetId(const char *msg)
static uint8_t HTPHandleWarningGetId(const char *msg)
{
SCLogDebug("received warning \"%s\"", msg);
size_t idx;
Expand All @@ -618,7 +618,7 @@ static int HTPHandleWarningGetId(const char *msg)
*
* \retval id the id or 0 in case of not found
*/
static int HTPHandleErrorGetId(const char *msg)
static uint8_t HTPHandleErrorGetId(const char *msg)
{
SCLogDebug("received error \"%s\"", msg);

Expand Down Expand Up @@ -675,7 +675,7 @@ static void HTPHandleError(HtpState *s, const uint8_t dir)

SCLogDebug("message %s", log->msg);

int id = HTPHandleErrorGetId(log->msg);
uint8_t id = HTPHandleErrorGetId(log->msg);
if (id == 0) {
id = HTPHandleWarningGetId(log->msg);
if (id == 0)
Expand Down Expand Up @@ -1255,9 +1255,9 @@ static void HtpRequestBodyMultipartParseHeader(HtpState *hstate,
ft_len = USHRT_MAX;

*filename = fn;
*filename_len = fn_len;
*filename_len = (uint16_t)fn_len;
*filetype = ft;
*filetype_len = ft_len;
*filetype_len = (uint16_t)ft_len;
}

/**
Expand Down Expand Up @@ -1304,8 +1304,8 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
{
int result = 0;
uint8_t boundary[htud->boundary_len + 4]; /**< size limited to HTP_BOUNDARY_MAX + 4 */
uint32_t expected_boundary_len = htud->boundary_len + 2;
uint32_t expected_boundary_end_len = htud->boundary_len + 4;
uint16_t expected_boundary_len = htud->boundary_len + 2;
uint16_t expected_boundary_end_len = htud->boundary_len + 4;
int tx_progress = 0;

#ifdef PRINT
Expand Down Expand Up @@ -1434,7 +1434,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
/* skip empty records */
if (expected_boundary_len == header_len) {
goto next;
} else if ((expected_boundary_len + 2) <= header_len) {
} else if ((uint32_t)(expected_boundary_len + 2) <= header_len) {
header_len -= (expected_boundary_len + 2);
header = (uint8_t *)header_start + (expected_boundary_len + 2); // + for 0d 0a
}
Expand Down Expand Up @@ -1536,7 +1536,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud,
SCLogDebug("offset %u", offset);
htud->request_body.body_parsed += offset;

if (filedata_len >= (expected_boundary_len + 2)) {
if (filedata_len >= (uint32_t)(expected_boundary_len + 2)) {
filedata_len -= (expected_boundary_len + 2 - 1);
SCLogDebug("opening file with partial data");
} else {
Expand Down Expand Up @@ -1630,7 +1630,11 @@ static int HtpRequestBodyHandlePOSTorPUT(HtpState *hstate, HtpTxUserData *htud,
}

if (filename != NULL) {
result = HTPFileOpen(hstate, htud, filename, (uint32_t)filename_len, data, data_len,
if (filename_len > UINT16_MAX) {
// explicitly truncate the file name if too long
filename_len = UINT16_MAX;
}
result = HTPFileOpen(hstate, htud, filename, (uint16_t)filename_len, data, data_len,
HtpGetActiveRequestTxID(hstate), STREAM_TOSERVER);
if (result == -1) {
goto end;
Expand Down Expand Up @@ -1703,11 +1707,15 @@ static int HtpResponseBodyHandle(HtpState *hstate, HtpTxUserData *htud,
if (filename != NULL) {
// set range if present
htp_header_t *h_content_range = htp_table_get_c(tx->response_headers, "content-range");
if (filename_len > UINT16_MAX) {
// explicitly truncate the file name if too long
filename_len = UINT16_MAX;
}
if (h_content_range != NULL) {
result = HTPFileOpenWithRange(hstate, htud, filename, (uint32_t)filename_len, data,
result = HTPFileOpenWithRange(hstate, htud, filename, (uint16_t)filename_len, data,
data_len, HtpGetActiveResponseTxID(hstate), h_content_range->value, htud);
} else {
result = HTPFileOpen(hstate, htud, filename, (uint32_t)filename_len, data, data_len,
result = HTPFileOpen(hstate, htud, filename, (uint16_t)filename_len, data, data_len,
HtpGetActiveResponseTxID(hstate), STREAM_TOCLIENT);
}
SCLogDebug("result %d", result);
Expand Down Expand Up @@ -3025,7 +3033,7 @@ static int HTPRegisterPatternsForProtocolDetection(void)
* but the pattern matching should only be one char
*/
register_result = AppLayerProtoDetectPMRegisterPatternCI(IPPROTO_TCP, ALPROTO_HTTP1,
method_buffer, strlen(method_buffer) - 3, 0, STREAM_TOSERVER);
method_buffer, (uint16_t)strlen(method_buffer) - 3, 0, STREAM_TOSERVER);
if (register_result < 0) {
return -1;
}
Expand All @@ -3035,7 +3043,8 @@ static int HTPRegisterPatternsForProtocolDetection(void)
/* Loop through all the http verions patterns that are TO_CLIENT */
for (versions_pos = 0; versions[versions_pos]; versions_pos++) {
register_result = AppLayerProtoDetectPMRegisterPatternCI(IPPROTO_TCP, ALPROTO_HTTP1,
versions[versions_pos], strlen(versions[versions_pos]), 0, STREAM_TOCLIENT);
versions[versions_pos], (uint16_t)strlen(versions[versions_pos]), 0,
STREAM_TOCLIENT);
if (register_result < 0) {
return -1;
}
Expand Down
4 changes: 2 additions & 2 deletions src/app-layer-parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ AppLayerParserThreadCtx *AppLayerParserThreadCtxAlloc(void)
SCEnter();

AppProto alproto = 0;
int flow_proto = 0;
uint8_t flow_proto = 0;
AppLayerParserThreadCtx *tctx;

tctx = SCMalloc(sizeof(*tctx));
Expand All @@ -275,7 +275,7 @@ void AppLayerParserThreadCtxFree(AppLayerParserThreadCtx *tctx)
SCEnter();

AppProto alproto = 0;
int flow_proto = 0;
uint8_t flow_proto = 0;

for (flow_proto = 0; flow_proto < FLOW_PROTO_DEFAULT; flow_proto++) {
for (alproto = 0; alproto < ALPROTO_MAX; alproto++) {
Expand Down
2 changes: 1 addition & 1 deletion src/app-layer-register.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
typedef struct AppLayerParser {
const char *name;
const char *default_port;
int ip_proto;
uint8_t ip_proto;

ProbingParserFPtr ProbeTS;
ProbingParserFPtr ProbeTC;
Expand Down
31 changes: 19 additions & 12 deletions src/app-layer-smtp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,12 @@ int SMTPProcessDataChunk(const uint8_t *chunk, uint32_t len,
SCLogDebug("StreamTcpReassemblySetMinInspectDepth STREAM_TOSERVER %"PRIu32, depth);
StreamTcpReassemblySetMinInspectDepth(flow->protoctx, STREAM_TOSERVER, depth);

uint16_t flen = (uint16_t)entity->filename_len;
if (entity->filename_len > UINT16_MAX) {
flen = UINT16_MAX;
}
if (FileOpenFileWithId(files, &smtp_config.sbcfg, smtp_state->file_track_id++,
(uint8_t *) entity->filename, entity->filename_len,
(uint8_t *) chunk, len, flags) != 0) {
(uint8_t *)entity->filename, flen, (uint8_t *)chunk, len, flags) != 0) {
ret = MIME_DEC_ERR_DATA;
SCLogDebug("FileOpenFile() failed");
}
Expand Down Expand Up @@ -1154,7 +1157,11 @@ static int SMTPParseCommandWithParam(SMTPState *state, uint8_t prefix_len, uint8
return -1;
memcpy(*target, state->current_line + i, spc_i - i);
(*target)[spc_i - i] = '\0';
*target_len = spc_i - i;
if (spc_i - i > UINT16_MAX) {
*target_len = UINT16_MAX;
} else {
*target_len = (uint16_t)(spc_i - i);
}

return 0;
}
Expand Down Expand Up @@ -1215,6 +1222,9 @@ static int NoNewTx(SMTPState *state)
return 0;
}

/* XXX have a better name */
#define rawmsgname "rawmsg"

static int SMTPProcessRequest(SMTPState *state, Flow *f,
AppLayerParserState *pstate)
{
Expand Down Expand Up @@ -1255,7 +1265,6 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f,
SCMemcmpLowercase("data", state->current_line, 4) == 0) {
state->current_command = SMTP_COMMAND_DATA;
if (smtp_config.raw_extraction) {
const char *msgname = "rawmsg"; /* XXX have a better name */
if (state->files_ts == NULL)
state->files_ts = FileContainerAlloc();
if (state->files_ts == NULL) {
Expand All @@ -1272,10 +1281,9 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f,
TAILQ_INSERT_TAIL(&state->tx_list, tx, next);
tx->tx_id = state->tx_cnt++;
}
if (FileOpenFileWithId(state->files_ts, &smtp_config.sbcfg,
state->file_track_id++,
(uint8_t*) msgname, strlen(msgname), NULL, 0,
FILE_NOMD5|FILE_NOMAGIC|FILE_USE_DETECT) == 0) {
if (FileOpenFileWithId(state->files_ts, &smtp_config.sbcfg, state->file_track_id++,
(uint8_t *)rawmsgname, strlen(rawmsgname), NULL, 0,
FILE_NOMD5 | FILE_NOMAGIC | FILE_USE_DETECT) == 0) {
SMTPNewFile(state->curr_tx, state->files_ts->tail);
}
} else if (smtp_config.decode_mime) {
Expand Down Expand Up @@ -1378,10 +1386,9 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f,
}
}

static AppLayerResult SMTPParse(int direction, Flow *f, SMTPState *state,
AppLayerParserState *pstate, const uint8_t *input,
uint32_t input_len,
SMTPThreadCtx *thread_data)
static AppLayerResult SMTPParse(uint8_t direction, Flow *f, SMTPState *state,
AppLayerParserState *pstate, const uint8_t *input, uint32_t input_len,
SMTPThreadCtx *thread_data)
{
SCEnter();

Expand Down
Loading