From c4d9cb02ec5521bf051ebef6d0fcd986b6cc3b51 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Sat, 16 Apr 2022 15:51:29 +0200 Subject: [PATCH] util: better hex print function Without dangerous snprintf pattern identified by CodeQL even if this pattern is not a problem in those precise cases, it may easily get copy pasted in a dangerous place, so better get rid of it and make CodeQL happy --- rust/src/common.rs | 57 +++++++++++++++++++++++++++++----- rust/src/jsonbuilder.rs | 42 ++++++++++++++++++++++++- src/app-layer-ssl.c | 7 ++--- src/datasets.c | 12 ++----- src/output-filestore.c | 9 ------ src/output-json-email-common.c | 8 +---- src/output-json.c | 24 ++------------ src/util-print.c | 8 +++++ src/util-print.h | 1 + 9 files changed, 108 insertions(+), 60 deletions(-) diff --git a/rust/src/common.rs b/rust/src/common.rs index bbb4a73bfc..dadce7b2cb 100644 --- a/rust/src/common.rs +++ b/rust/src/common.rs @@ -1,3 +1,5 @@ +use super::build_slice; +use crate::jsonbuilder::HEX; use std::ffi::CString; use std::os::raw::c_char; @@ -12,9 +14,9 @@ pub mod nom7 { /// `take_until` does not consume the matched tag, and /// `take_until_and_consume` was removed in nom 7. This function /// provides an implementation (specialized for `&[u8]`). - pub fn take_until_and_consume<'a, E: ParseError<&'a [u8]>>(t: &'a [u8]) - -> impl Fn(&'a [u8]) -> IResult<&'a [u8], &'a [u8], E> - { + pub fn take_until_and_consume<'a, E: ParseError<&'a [u8]>>( + t: &'a [u8], + ) -> impl Fn(&'a [u8]) -> IResult<&'a [u8], &'a [u8], E> { move |i: &'a [u8]| { let (i, res) = take_until(t)(i)?; let (i, _) = tag(t)(i)?; @@ -115,9 +117,50 @@ pub unsafe extern "C" fn rs_cstring_free(s: *mut c_char) { /// Convert an u8-array of data into a hexadecimal representation pub fn to_hex(input: &[u8]) -> String { - static CHARS: &'static [u8] = b"0123456789abcdef"; + return input + .iter() + .map(|b| { + vec![ + char::from(HEX[(b >> 4) as usize]), + char::from(HEX[(b & 0xf) as usize]), + ] + }) + .flatten() + .collect(); +} + +#[no_mangle] +pub unsafe extern "C" fn rs_to_hex( + output: *mut u8, out_len: usize, input: *const u8, in_len: usize, +) { + if out_len < 2 * in_len + 1 { + return; + } + let islice = build_slice!(input, in_len); + let oslice = std::slice::from_raw_parts_mut(output, 2 * in_len + 1); + // only used from C + for i in 0..islice.len() { + oslice[2 * i] = HEX[(islice[i] >> 4) as usize]; + oslice[2 * i + 1] = HEX[(islice[i] & 0xf) as usize]; + } + oslice[2 * islice.len()] = 0; +} - return input.iter().map( - |b| vec![char::from(CHARS[(b >> 4) as usize]), char::from(CHARS[(b & 0xf) as usize])] - ).flatten().collect(); +#[no_mangle] +pub unsafe extern "C" fn rs_to_hex_sep( + output: *mut u8, out_len: usize, sep: u8, input: *const u8, in_len: usize, +) { + if out_len < 3 * in_len { + return; + } + let islice = build_slice!(input, in_len); + let oslice = std::slice::from_raw_parts_mut(output, 3 * in_len); + // only used from C + for i in 0..islice.len() { + oslice[3 * i] = HEX[(islice[i] >> 4) as usize]; + oslice[3 * i + 1] = HEX[(islice[i] & 0xf) as usize]; + oslice[3 * i + 2] = sep; + } + // overwrites last separator with final null char + oslice[3 * islice.len() - 1] = 0; } diff --git a/rust/src/jsonbuilder.rs b/rust/src/jsonbuilder.rs index 993186c9ee..dc7236c42c 100644 --- a/rust/src/jsonbuilder.rs +++ b/rust/src/jsonbuilder.rs @@ -474,6 +474,32 @@ impl JsonBuilder { Ok(self) } + /// Set a key and a string field as the hex encoded string of the value. + pub fn set_hex(&mut self, key: &str, val: &[u8]) -> Result<&mut Self, JsonError> { + match self.current_state() { + State::ObjectNth => { + self.buf.push(','); + } + State::ObjectFirst => { + self.set_state(State::ObjectNth); + } + _ => { + debug_validate_fail!("invalid state"); + return Err(JsonError::InvalidState); + } + } + self.buf.push('"'); + self.buf.push_str(key); + self.buf.push_str("\":\""); + for i in 0..val.len() { + self.buf.push(HEX[(val[i] >> 4) as usize] as char); + self.buf.push(HEX[(val[i] & 0xf) as usize] as char); + } + self.buf.push('"'); + + Ok(self) + } + /// Set a key and an unsigned integer type on an object. pub fn set_uint(&mut self, key: &str, val: u64) -> Result<&mut Self, JsonError> { match self.current_state() { @@ -716,6 +742,20 @@ pub unsafe extern "C" fn jb_set_base64( return false; } +#[no_mangle] +pub unsafe extern "C" fn jb_set_hex( + js: &mut JsonBuilder, key: *const c_char, bytes: *const u8, len: u32, +) -> bool { + if bytes == std::ptr::null() || len == 0 { + return false; + } + if let Ok(key) = CStr::from_ptr(key).to_str() { + let val = std::slice::from_raw_parts(bytes, len as usize); + return js.set_hex(key, val).is_ok(); + } + return false; +} + #[no_mangle] pub unsafe extern "C" fn jb_set_formatted(js: &mut JsonBuilder, formatted: *const c_char) -> bool { if let Ok(formatted) = CStr::from_ptr(formatted).to_str() { @@ -1160,6 +1200,6 @@ static ESCAPED: [u8; 256] = [ __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, // F ]; -static HEX: [u8; 16] = [ +pub static HEX: [u8; 16] = [ b'0', b'1', b'2', b'3', b'4', b'5', b'6', b'7', b'8', b'9', b'a', b'b', b'c', b'd', b'e', b'f', ]; diff --git a/src/app-layer-ssl.c b/src/app-layer-ssl.c index 2013a6cc96..4f48cd1ad4 100644 --- a/src/app-layer-ssl.c +++ b/src/app-layer-ssl.c @@ -466,11 +466,8 @@ static inline int TlsDecodeHSCertificateFingerprint(SSLState *ssl_state, uint8_t hash[SC_SHA1_LEN]; if (SCSha1HashBuffer(input, cert_len, hash, sizeof(hash)) == 1) { - for (int i = 0, x = 0; x < SC_SHA1_LEN; x++) { - i += snprintf(ssl_state->server_connp.cert0_fingerprint + i, - SHA1_STRING_LENGTH - i, i == 0 ? "%02x" : ":%02x", - hash[x]); - } + rs_to_hex_sep((uint8_t *)ssl_state->server_connp.cert0_fingerprint, SHA1_STRING_LENGTH, ':', + hash, SC_SHA1_LEN); } return 0; } diff --git a/src/datasets.c b/src/datasets.c index 8b20aa45e2..a83bafd412 100644 --- a/src/datasets.c +++ b/src/datasets.c @@ -766,12 +766,8 @@ static int SaveCallback(void *ctx, const uint8_t *data, const uint32_t data_len) static int Md5AsAscii(const void *s, char *out, size_t out_size) { const Md5Type *md5 = s; - uint32_t x; - int i; char str[256]; - for (i = 0, x = 0; x < sizeof(md5->md5); x++) { - i += snprintf(&str[i], 255-i, "%02x", md5->md5[x]); - } + PrintHexString(str, sizeof(str), (uint8_t *)md5->md5, sizeof(md5->md5)); strlcat(out, str, out_size); strlcat(out, "\n", out_size); return strlen(out); @@ -780,12 +776,8 @@ static int Md5AsAscii(const void *s, char *out, size_t out_size) static int Sha256AsAscii(const void *s, char *out, size_t out_size) { const Sha256Type *sha = s; - uint32_t x; - int i; char str[256]; - for (i = 0, x = 0; x < sizeof(sha->sha256); x++) { - i += snprintf(&str[i], 255-i, "%02x", sha->sha256[x]); - } + PrintHexString(str, sizeof(str), (uint8_t *)sha->sha256, sizeof(sha->sha256)); strlcat(out, str, out_size); strlcat(out, "\n", out_size); return strlen(out); diff --git a/src/output-filestore.c b/src/output-filestore.c index 227638f173..e45142d8c0 100644 --- a/src/output-filestore.c +++ b/src/output-filestore.c @@ -88,15 +88,6 @@ static uint32_t FileGetMaxOpenFiles(void) return g_file_store_max_open_files; } -static void PrintHexString(char *str, size_t size, uint8_t *buf, size_t buf_len) -{ - int i = 0; - size_t x = 0; - for (i = 0, x = 0; x < buf_len; x++) { - i += snprintf(&str[i], size - i, "%02x", buf[x]); - } -} - /** * \brief Update the timestamps on a file to match those of another * file. diff --git a/src/output-json-email-common.c b/src/output-json-email-common.c index 613421eb80..5d110c0ea3 100644 --- a/src/output-json-email-common.c +++ b/src/output-json-email-common.c @@ -138,13 +138,7 @@ static void EveEmailLogJSONMd5(OutputJsonEmailCtx *email_ctx, JsonBuilder *js, S if (email_ctx->flags & LOG_EMAIL_BODY_MD5) { MimeDecParseState *mime_state = tx->mime_state; if (mime_state && mime_state->has_md5 && (mime_state->state_flag == PARSE_DONE)) { - size_t x; - int i; - char s[256]; - for (i = 0, x = 0; x < sizeof(mime_state->md5); x++) { - i += snprintf(s + i, 255 - i, "%02x", mime_state->md5[x]); - } - jb_set_string(js, "body_md5", s); + jb_set_hex(js, "body_md5", mime_state->md5, (uint32_t)sizeof(mime_state->md5)); } } } diff --git a/src/output-json.c b/src/output-json.c index b1fb4acf5b..4a89c1030b 100644 --- a/src/output-json.c +++ b/src/output-json.c @@ -143,22 +143,10 @@ void EveFileInfo(JsonBuilder *jb, const File *ff, const bool stored) case FILE_STATE_CLOSED: JB_SET_STRING(jb, "state", "CLOSED"); if (ff->flags & FILE_MD5) { - size_t x; - int i; - char str[256]; - for (i = 0, x = 0; x < sizeof(ff->md5); x++) { - i += snprintf(&str[i], 255-i, "%02x", ff->md5[x]); - } - jb_set_string(jb, "md5", str); + jb_set_hex(jb, "md5", (uint8_t *)ff->md5, (uint32_t)sizeof(ff->md5)); } if (ff->flags & FILE_SHA1) { - size_t x; - int i; - char str[256]; - for (i = 0, x = 0; x < sizeof(ff->sha1); x++) { - i += snprintf(&str[i], 255-i, "%02x", ff->sha1[x]); - } - jb_set_string(jb, "sha1", str); + jb_set_hex(jb, "sha1", (uint8_t *)ff->sha1, (uint32_t)sizeof(ff->sha1)); } break; case FILE_STATE_TRUNCATED: @@ -173,13 +161,7 @@ void EveFileInfo(JsonBuilder *jb, const File *ff, const bool stored) } if (ff->flags & FILE_SHA256) { - size_t x; - int i; - char str[256]; - for (i = 0, x = 0; x < sizeof(ff->sha256); x++) { - i += snprintf(&str[i], 255-i, "%02x", ff->sha256[x]); - } - jb_set_string(jb, "sha256", str); + jb_set_hex(jb, "sha256", (uint8_t *)ff->sha256, (uint32_t)sizeof(ff->sha256)); } if (stored) { diff --git a/src/util-print.c b/src/util-print.c index cd1da1db7f..efef902163 100644 --- a/src/util-print.c +++ b/src/util-print.c @@ -27,6 +27,8 @@ #include "util-print.h" #include "util-error.h" #include "util-debug.h" +#include "util-validate.h" +#include "rust.h" /** * \brief print a buffer as hex on a single line @@ -292,3 +294,9 @@ const char *PrintInet(int af, const void *src, char *dst, socklen_t size) } return NULL; } + +void PrintHexString(char *str, size_t size, uint8_t *buf, size_t buf_len) +{ + DEBUG_VALIDATE_BUG_ON(size < 2 * buf_len); + rs_to_hex((uint8_t *)str, size, buf, buf_len); +} diff --git a/src/util-print.h b/src/util-print.h index 6548f02197..249ec20f73 100644 --- a/src/util-print.h +++ b/src/util-print.h @@ -53,6 +53,7 @@ void PrintStringsToBuffer(uint8_t *dst_buf, uint32_t *dst_buf_offset_ptr, uint32 const uint8_t *src_buf, const uint32_t src_buf_len); void PrintRawLineHexBuf(char *, uint32_t, const uint8_t *, uint32_t ); const char *PrintInet(int , const void *, char *, socklen_t); +void PrintHexString(char *str, size_t size, uint8_t *buf, size_t buf_len); #endif /* __UTIL_PRINT_H__ */