Skip to content

Commit 8b9704b

Browse files
Merge pull request #13 from cowrie/claude/review-c-code-logic-gKpMn
2 parents 55c3c1f + 01f85af commit 8b9704b

File tree

13 files changed

+88
-72
lines changed

13 files changed

+88
-72
lines changed

modules/curl/module.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,7 @@ static int curl_socketfunction_cb(CURL *easy, curl_socket_t s, int action, void
306306
struct session *session;
307307
curl_easy_getinfo(easy, CURLINFO_PRIVATE, (char **)&session);
308308

309-
#ifdef DEBUG
310309
const char *action_str[]={ "none", "IN", "OUT", "INOUT", "REMOVE"};
311-
#endif
312310

313311
g_debug("socket callback: s=%d e=%p what=%s ", s, easy, action_str[action]);
314312

@@ -353,7 +351,7 @@ static size_t curl_writefunction_cb(void *ptr, size_t size, size_t nmemb, void *
353351
return size*nmemb;
354352

355353
g_debug("session %p file %i", session, session->action.upload.file->fd);
356-
if( write(session->action.upload.file->fd, ptr, size*nmemb) != size*nmemb)
354+
if( write(session->action.upload.file->fd, ptr, size*nmemb) != (ssize_t)(size*nmemb))
357355
return 0;
358356
}
359357

modules/emu/detect.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ void proc_emu_on_io_in(struct connection *con, struct processor_data *pd)
133133
#endif
134134
ret = emu_shellcode_test(e, streamdata, size);
135135
emu_free(e);
136-
ctx->offset += size;
136+
ctx->offset = offset + size;
137137
if( ret >= 0 )
138138
{
139139
struct incident *ix = incident_new("dionaea.shellcode.detected");

modules/emu/emulate.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,8 @@ void emulate_ctx_free(void *data)
195195
if( ctx->time != NULL )
196196
g_timer_destroy(ctx->time);
197197

198-
emu_free(ctx->emu);
199198
emu_env_free(ctx->env);
199+
emu_free(ctx->emu);
200200
g_mutex_clear(&ctx->mutex);
201201
if( ctx->ctxcon != NULL )
202202
connection_unref(ctx->ctxcon);

modules/nfq/nfq.c

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,18 +188,31 @@ static int nfqueue_cb(struct nfq_q_handle *qh, struct nfgenmsg *nfmsg, struct nf
188188
return 1; // from nfqueue source: 0 = ok, >0 = soft error, <0 hard error
189189
}
190190

191+
id = ntohl(ph->packet_id);
192+
191193
len = nfq_get_payload(nfa, &payload);
192194

193195
if( len <= 0 )
194-
return 0;
196+
{
197+
nf = NF_DROP;
198+
goto send_verdict;
199+
}
195200

196-
if( len <= sizeof(struct iphdr) )
197-
return 0;
201+
if( len <= (int)sizeof(struct iphdr) )
202+
{
203+
nf = NF_DROP;
204+
goto send_verdict;
205+
}
198206

199207
struct iphdr * ip = (struct iphdr *) payload;
200208
if( ip->version == IPVERSION )
201209
{ /* IPv4 */
202-
if( len >= ip->ihl * 4 + sizeof(struct tcphdr) )
210+
if( ip->ihl < 5 )
211+
{
212+
nf = NF_DROP;
213+
goto send_verdict;
214+
}
215+
if( len >= ip->ihl * 4 + (int)sizeof(struct tcphdr) )
203216
{
204217
struct tcphdr * tcp = (struct tcphdr *) (payload + ip->ihl * 4);
205218

@@ -233,7 +246,8 @@ static int nfqueue_cb(struct nfq_q_handle *qh, struct nfgenmsg *nfmsg, struct nf
233246
nf = NF_ACCEPT;
234247
}
235248

236-
id = ntohl(ph->packet_id);
249+
send_verdict:
250+
;
237251
uintptr_t cmd = (uintptr_t)nfq_backend;
238252
if( send(g_dionaea->pchild->fd, &cmd, sizeof(uintptr_t), 0) != sizeof(uintptr_t) ||
239253
send(g_dionaea->pchild->fd, &id, sizeof(id), 0) != sizeof(id) ||

modules/pcap/pcap.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,11 +257,11 @@ static bool pcap_prepare(void)
257257
if( addr->addr )
258258
g_debug("\t\t\taddr %s", inet_ntop(addr->addr->sa_family, addr_offset(addr->addr), name, 128));
259259
if( addr->netmask )
260-
g_debug("\t\t\tnetmask %s", inet_ntop(addr->addr->sa_family, addr_offset(addr->addr), name, 128));
260+
g_debug("\t\t\tnetmask %s", inet_ntop(addr->netmask->sa_family, addr_offset(addr->netmask), name, 128));
261261
if( addr->broadaddr )
262-
g_debug("\t\t\tbcast %s", inet_ntop(addr->addr->sa_family, addr_offset(addr->addr), name, 128));
262+
g_debug("\t\t\tbcast %s", inet_ntop(addr->broadaddr->sa_family, addr_offset(addr->broadaddr), name, 128));
263263
if( addr->dstaddr )
264-
g_debug("\t\t\tdstaddr %s", inet_ntop(addr->addr->sa_family, addr_offset(addr->addr), name, 128));
264+
g_debug("\t\t\tdstaddr %s", inet_ntop(addr->dstaddr->sa_family, addr_offset(addr->dstaddr), name, 128));
265265
g_string_append_printf(bpf_filter_string_addition, "or src host %s ", inet_ntop(addr->addr->sa_family, addr_offset(addr->addr), name, 128));
266266
break;
267267

modules/python/dionaea/hpfeeds.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,18 @@ def timestr():
7272
def strpack8(x):
7373
if isinstance(x, str):
7474
x = x.encode('latin1')
75-
return struct.pack('!B', len(x)%0xff) + x
75+
if len(x) > 255:
76+
raise BadClient('strpack8: string too long (%d bytes)' % len(x))
77+
return struct.pack('!B', len(x)) + x
7678

7779

7880
# unpacks a string with 1 byte length field
7981
def strunpack8(x):
82+
if len(x) < 1:
83+
raise BadClient('strunpack8: buffer too short for length field')
8084
length = x[0]
85+
if len(x) < 1 + length:
86+
raise BadClient('strunpack8: buffer too short (need %d, have %d)' % (1 + length, len(x)))
8187
return x[1:1+length], x[1+length:]
8288

8389

modules/python/module.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ static bool hupy(void)
197197
}
198198
for (module_name = module_names; *module_name; module_name++) {
199199
struct import *i;
200-
if( (i = g_hash_table_lookup(runtime.imports, module_name)) != NULL ) {
200+
if( (i = g_hash_table_lookup(runtime.imports, *module_name)) != NULL ) {
201201
g_message("Import %s exists, reloading", *module_name);
202202

203203
PyObject *func = PyObject_GetAttrString(i->module, "stop");
@@ -670,6 +670,7 @@ PyObject *pygetifaddrs(PyObject *self, PyObject *args)
670670
pyiface = PyUnicode_FromString(iface->ifa_name);
671671
pyafdict = PyDict_New();
672672
PyDict_SetItemString (result, iface->ifa_name, pyafdict);
673+
Py_DECREF(pyafdict);
673674
Py_DECREF(pyiface);
674675
}
675676

@@ -678,6 +679,7 @@ PyObject *pygetifaddrs(PyObject *self, PyObject *args)
678679
{
679680
pyaflist = PyList_New(0);
680681
PyDict_SetItem(pyafdict, pyaf, pyaflist);
682+
Py_DECREF(pyaflist);
681683
} else
682684
{
683685
pyaflist = PyDict_GetItem(pyafdict, pyaf);
@@ -862,22 +864,30 @@ PyObject *py_config(PyObject *self, PyObject *args)
862864
obj2 = PyDict_New();
863865
obj_value = py_config_string("dionaea", "listen.mode");
864866
PyDict_SetItemString(obj2, "listen.mode", obj_value);
867+
Py_XDECREF(obj_value);
865868
obj_value = py_config_string_list("dionaea", "listen.interfaces");
866869
PyDict_SetItemString(obj2, "listen.interfaces", obj_value);
870+
Py_XDECREF(obj_value);
867871
obj_value = py_config_string_list("dionaea", "listen.addresses");
868872
PyDict_SetItemString(obj2, "listen.addresses", obj_value);
873+
Py_XDECREF(obj_value);
869874
obj_value = py_config_string("dionaea", "download.dir");
870875
PyDict_SetItemString(obj2, "download.dir", obj_value);
876+
Py_XDECREF(obj_value);
871877

872878
PyDict_SetItemString(obj, "dionaea", obj2);
879+
Py_DECREF(obj2);
873880

874881
obj2 = PyDict_New();
875882
obj_value = py_config_string_list("module.python", "ihandler_configs");
876883
PyDict_SetItemString(obj2, "ihandler_configs", obj_value);
884+
Py_XDECREF(obj_value);
877885
obj_value = py_config_string_list("module.python", "service_configs");
878886
PyDict_SetItemString(obj2, "service_configs", obj_value);
887+
Py_XDECREF(obj_value);
879888

880889
PyDict_SetItemString(obj, "module", obj2);
890+
Py_DECREF(obj2);
881891

882892
return obj;
883893
}

modules/speakeasy/detect.c

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -48,36 +48,44 @@ void speakeasy_set_shellcode_dir(const char *dir)
4848
shellcode_dir = g_strdup(dir);
4949
}
5050

51+
// SHA256 hex string is always 64 chars + null
52+
#define SHA256_HEX_SIZE 65
53+
// Max path: dir + "/shellcode-" + 64 hex chars + ".bin"/".txt" + null
54+
#define SHELLCODE_PATH_MAX 512
55+
5156
/**
52-
* Compute SHA256 hash of data and return as hex string
53-
* Caller must free the returned string with g_free()
57+
* Compute SHA256 hash of data and write hex string into caller-provided buffer
58+
* Buffer must be at least SHA256_HEX_SIZE (65) bytes
59+
* Returns true on success, false on failure
5460
*/
55-
static char *compute_sha256_hex(const void *data, size_t len)
61+
static bool compute_sha256_hex(char *hex, size_t hex_size, const void *data, size_t len)
5662
{
5763
unsigned char hash[EVP_MAX_MD_SIZE];
5864
unsigned int hash_len;
5965

66+
if (hex_size < SHA256_HEX_SIZE)
67+
return false;
68+
6069
EVP_MD_CTX *ctx = EVP_MD_CTX_new();
6170
if (ctx == NULL) {
62-
return NULL;
71+
return false;
6372
}
6473

6574
if (EVP_DigestInit_ex(ctx, EVP_sha256(), NULL) != 1 ||
6675
EVP_DigestUpdate(ctx, data, len) != 1 ||
6776
EVP_DigestFinal_ex(ctx, hash, &hash_len) != 1) {
6877
EVP_MD_CTX_free(ctx);
69-
return NULL;
78+
return false;
7079
}
7180
EVP_MD_CTX_free(ctx);
7281

7382
// Convert to hex string
74-
char *hex = g_malloc(hash_len * 2 + 1);
7583
for (unsigned int i = 0; i < hash_len; i++) {
7684
snprintf(hex + i * 2, 3, "%02x", hash[i]);
7785
}
7886
hex[hash_len * 2] = '\0';
7987

80-
return hex;
88+
return true;
8189
}
8290

8391
/**
@@ -97,33 +105,37 @@ static bool save_shellcode(const void *data, size_t len, int offset,
97105
return false;
98106
}
99107

100-
char *sha256 = compute_sha256_hex(data, len);
101-
if (sha256 == NULL) {
108+
char sha256[SHA256_HEX_SIZE];
109+
if (!compute_sha256_hex(sha256, sizeof(sha256), data, len)) {
102110
g_warning("Failed to compute SHA256");
103111
return false;
104112
}
105113

106-
// Build file paths
107-
char *bin_path = g_strdup_printf("%s/shellcode-%s.bin", shellcode_dir, sha256);
108-
char *txt_path = g_strdup_printf("%s/shellcode-%s.txt", shellcode_dir, sha256);
114+
// Build file paths on stack
115+
char bin_path[SHELLCODE_PATH_MAX];
116+
char txt_path[SHELLCODE_PATH_MAX];
117+
int n = snprintf(bin_path, sizeof(bin_path), "%s/shellcode-%s.bin", shellcode_dir, sha256);
118+
if (n < 0 || (size_t)n >= sizeof(bin_path)) {
119+
g_warning("Shellcode bin path too long");
120+
return false;
121+
}
122+
n = snprintf(txt_path, sizeof(txt_path), "%s/shellcode-%s.txt", shellcode_dir, sha256);
123+
if (n < 0 || (size_t)n >= sizeof(txt_path)) {
124+
g_warning("Shellcode txt path too long");
125+
return false;
126+
}
109127

110128
// Check if already exists (dedup by hash)
111129
struct stat st;
112130
if (stat(bin_path, &st) == 0) {
113131
g_debug("Shellcode %s already saved", sha256);
114-
g_free(sha256);
115-
g_free(bin_path);
116-
g_free(txt_path);
117132
return true;
118133
}
119134

120135
// Save binary data
121136
FILE *f = fopen(bin_path, "wb");
122137
if (f == NULL) {
123138
g_warning("Failed to open %s for writing", bin_path);
124-
g_free(sha256);
125-
g_free(bin_path);
126-
g_free(txt_path);
127139
return false;
128140
}
129141
size_t written = fwrite(data, 1, len, f);
@@ -132,9 +144,6 @@ static bool save_shellcode(const void *data, size_t len, int offset,
132144
if (written != len) {
133145
g_warning("Failed to write shellcode data");
134146
unlink(bin_path);
135-
g_free(sha256);
136-
g_free(bin_path);
137-
g_free(txt_path);
138147
return false;
139148
}
140149

@@ -166,9 +175,6 @@ static bool save_shellcode(const void *data, size_t len, int offset,
166175

167176
g_info("Saved shellcode %s.bin (%zu bytes, %s)", sha256, len, arch);
168177

169-
g_free(sha256);
170-
g_free(bin_path);
171-
g_free(txt_path);
172178
return true;
173179
}
174180

@@ -484,14 +490,14 @@ void proc_speakeasy_on_io_in(struct connection *con, struct processor_data *pd)
484490
// All architectures now use execution-based validation to reduce false positives
485491

486492
// x86-32 uses libemu for emulation-based detection
487-
struct emu *e = emu_new();
488493
if( size <= 0 )
489494
{
490495
g_debug("No data to scan (size=%d)", size);
496+
g_free(streamdata);
491497
return;
492498
}
493-
uint16_t scan_size = (size > UINT16_MAX) ? UINT16_MAX : (uint16_t)size;
494-
int ret_x86 = emu_shellcode_test_x86(e, streamdata, scan_size);
499+
struct emu *e = emu_new();
500+
int ret_x86 = emu_shellcode_test_x86(e, streamdata, (uint32_t)size);
495501
emu_free(e);
496502

497503
// x86-64 still uses pattern-based detection (TODO: add execution validation)

modules/xmatch/xmatch.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ void proc_xmatch_on_io_in(struct connection *con, struct processor_data *pd)
192192
return;
193193
case 0:
194194
g_debug("did not find any matches.");
195+
g_free(streamdata);
195196
return;
196197
}
197198

src/bistream.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,9 +287,11 @@ int32_t bistream_get_stream(struct bistream *bs, enum bistream_direction dir, ui
287287
g_debug("found stream begin %p stream_offset %i size %i", itsc, itsc->stream_offset, (int)itsc->data->len);
288288

289289
uint32_t offset = 0;
290-
while( itsc->stream_offset < end )
290+
while( 1 )
291291
{
292292
itsc = it->data;
293+
if( itsc->stream_offset >= (uint32_t)end )
294+
break;
293295
uint32_t start_offset = 0;
294296
uint32_t end_offset = itsc->data->len;
295297

0 commit comments

Comments
 (0)