Skip to content

Commit 35c869f

Browse files
committed
rjv3: improve reliability when parsing properties
According to issues, there are more than expected cases where a property has inaccurate length fields, which could confuse the parser and lead to failure finding the echo key. For example, #18 pointed out that a property with header2.type == 0 could have a undersized length field, and #58 found one with an invalid length which is shorter than the header itself (and caused potential out-of-bound access in the parser). This commit addresses all known cases by far.
1 parent e0a9882 commit 35c869f

File tree

1 file changed

+23
-9
lines changed

1 file changed

+23
-9
lines changed

packet_plugin/rjv3/packet_plugin_rjv3_prop.c

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,26 @@ static uint8_t* find_byte_pattern(uint8_t* pattern, int patlen, uint8_t* buf, in
165165
return NULL;
166166
}
167167

168-
RESULT parse_rjv3_buf_to_prop_list(LIST_ELEMENT** list, uint8_t* buf, int buflen, int bare) {
168+
/* Does this prop has a reliable prop->header2.len?
169+
* If not, we need to perform a pattern search for next header, instead of
170+
* fast-forwarding header2.len bytes.
171+
*/
172+
static inline int rjv3_prop_length_field_ok(RJ_PROP *prop) {
173+
/* These two types of property are known unreliable, see https://github.com/updateing/minieap/issues/18 */
174+
if (prop->header2.type == 0 || prop->header2.type == 1) {
175+
return 0;
176+
}
177+
178+
/* header2.len needs to cover itself at least. If it's not, we won't be able to trust it. */
179+
if (prop->header2.len < sizeof(RJ_PROP_HEADER2) - sizeof(prop->header2.magic)) {
180+
PR_DBG("类型为 0x%02hhx 的字段中,长度信息未涵盖头部自身", prop->header2.type);
181+
return 0;
182+
}
183+
184+
return 1;
185+
}
186+
187+
RESULT parse_rjv3_buf_to_prop_list(LIST_ELEMENT** list, uint8_t* buf, int buflen, int bare /* buf_has_header1 ? */) {
169188
int _read_len = 0, _content_len = 0;
170189
uint8_t _magic[] = {0x00, 0x00, 0x13, 0x11};
171190
RJ_PROP* _tmp_prop = new_rjv3_prop();
@@ -182,17 +201,12 @@ RESULT parse_rjv3_buf_to_prop_list(LIST_ELEMENT** list, uint8_t* buf, int buflen
182201

183202
if (memcmp(_tmp_prop->header2.magic, _magic, sizeof(_magic)) == 0) {
184203
/* Valid */
185-
if (_tmp_prop->header2.type == 0) {
186-
/* 0x0 prop's len does not include HEADER2 */
187-
_content_len = _tmp_prop->header2.len;
188-
} else if (_tmp_prop->header2.type == 1) {
189-
/* Type 0x1 means there is no length info, we have to search for next 00 00 13 11 */
204+
if (rjv3_prop_length_field_ok(_tmp_prop)) {
205+
_content_len = _tmp_prop->header2.len - sizeof(RJ_PROP_HEADER2) + sizeof(_tmp_prop->header2.magic);
206+
} else {
190207
uint8_t* _next_magic = find_byte_pattern(_magic, sizeof(_magic),
191208
buf + _read_len, buflen - _read_len);
192209
_content_len = _next_magic ? (_next_magic - (buf + _read_len)) : buflen - _read_len;
193-
} else {
194-
/* This is normal prop, len includes two bytes from HEADER2 */
195-
_content_len = _tmp_prop->header2.len - sizeof(RJ_PROP_HEADER2) + sizeof(_tmp_prop->header2.magic);
196210
}
197211

198212
append_rjv3_prop(list,

0 commit comments

Comments
 (0)