Skip to content

Commit

Permalink
Werror, some more checks
Browse files Browse the repository at this point in the history
  • Loading branch information
DavidBuchanan314 committed Dec 29, 2024
1 parent 7bb3b1b commit eb4ef08
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 5 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,6 @@ print(decoded) # zb2rhZfjRh2FHHB2RkHVEvL2vJnCTcu7kwRqgVsf9gpkLgteo

```sh
# clone the repo
python3 -m pip install -e .
python3 -m pip install -ve .
python3 -m unittest -v
```
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
Extension(
"cbrrr._cbrrr",
sources=["src/cbrrr/_cbrrr.c"],
extra_compile_args=["-O3", "-Wall", "-Wextra", "-Wpedantic", "-std=c99"],
extra_compile_args=["-O3", "-Wall", "-Wextra", "-Wpedantic", "-std=c99", "-Werror"], # sorry, I hate Werror too, but this code is security-sensive and it's much better to have no build than to have an insecure build. please file a github issue if you're hitting this.
),
],
)
19 changes: 16 additions & 3 deletions src/cbrrr/_cbrrr.c
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,14 @@ cbrrr_parse_raw_string(const uint8_t *buf, size_t len, DCMajorType type, const u
// python error has been set by cbrrr_parse_minimal_varint
return -1;
}

// should only be plausible on 32-bit platforms
if (idx > SIZE_MAX - res) {
PyErr_SetString(PY_CBRRR_DECODE_ERROR, "index overflow");
return -1;
}
idx += res;

if (actual_str_len > (uint64_t)len - idx) { // should also handle cases where actual_str_len is > SIZE_MAX
PyErr_SetString(PY_CBRRR_DECODE_ERROR, "not enough bytes left in buffer");
return -1;
Expand Down Expand Up @@ -341,6 +348,12 @@ cbrrr_parse_token(const uint8_t *buf, size_t len, DCToken *token, PyObject *cid_
// python error set by cbrrr_parse_minimal_varint
return -1;
}

// should only be plausible on 32-bit platforms
if (idx > SIZE_MAX - res) {
PyErr_SetString(PY_CBRRR_DECODE_ERROR, "index overflow");
return -1;
}
idx += res;

// at this point, `info` represents its actual value, with meaning depending on the major type
Expand Down Expand Up @@ -392,7 +405,7 @@ cbrrr_parse_token(const uint8_t *buf, size_t len, DCToken *token, PyObject *cid_
}
return idx + info;
case DCMT_TEXT_STRING:
if (info > len - idx) {
if (info > (uint64_t)len - idx) {
PyErr_SetString(PY_CBRRR_DECODE_ERROR, "not enough bytes left in buffer");
return -1;
}
Expand All @@ -402,7 +415,7 @@ cbrrr_parse_token(const uint8_t *buf, size_t len, DCToken *token, PyObject *cid_
}
return idx + info;
case DCMT_ARRAY:
if (info > len - idx) {
if (info > (uint64_t)len - idx) {
PyErr_SetString(PY_CBRRR_DECODE_ERROR, "not enough bytes left in buffer for an array that long");
return -1;
}
Expand All @@ -413,7 +426,7 @@ cbrrr_parse_token(const uint8_t *buf, size_t len, DCToken *token, PyObject *cid_
token->count = info;
return idx;
case DCMT_MAP:
if (info > len - idx) {
if (info > (uint64_t)len - idx) {
PyErr_SetString(PY_CBRRR_DECODE_ERROR, "not enough bytes left in buffer for a map that long");
return -1;
}
Expand Down

0 comments on commit eb4ef08

Please sign in to comment.