On 28/09/17 22:02, Nils Freydank wrote:
> @@ -1067,22 +1101,38 @@ int SYMEXPORT alpm_extract_keyid(alpm_handle_t
> *handle, const char *identifier,
>
> spos = pos;
>
> + if(length_check(len, pos, ulen, handle, identifier)) {
> + return -1;
> + }
Why is this check needed? spos starts at pos, and we know pos is good
here. Every future usage of spos is checked.
> while(spos < pos + ulen) {
> if(sig[spos] < 192) {
> slen = sig[spos];
> + if(length_check(len + ulen, spos, 1, handle,
> identifier)) {
> + return -1;
> + }
Why is this check needed? We are not using this value without checking
it in the future.
> spos = spos + 1;
> } else if(sig[spos] < 255) {
> + if(length_check(pos + ulen, spos, 2, handle,
> identifier))
> + {
> + return -1;
> + }
> slen = (sig[spos] << 8) | sig[spos + 1];
> spos = spos + 2;
> } else {
> + /* check for pos and spos, as spos is still pos
> */
> + if(length_check(len, pos, 5, handle,
> identifier)) {
> + return -1;
> + }
> slen = (sig[spos + 1] << 24) | (sig[spos + 2]
> << 16) | (sig[spos + 3] << 8) | sig[spos + 4];
> spos = spos + 5;
> }
>
> if(sig[spos] == 16) {
> /* issuer key ID */
> char key[17];
> size_t i;
> + if(length_check(pos + ulen, spos, 8, handle,
> identifier)) {
> + return -1;
> + }
> for (i = 0; i < 8; i++) {
> sprintf(&key[i * 2], "%02X", sig[spos +
> i + 1]);
> }
> @@ -1090,12 +1140,16 @@ int SYMEXPORT alpm_extract_keyid(alpm_handle_t
> *handle, const char *identifier,
> break;
> }
>
> + if(length_check(pos + ulen + 1, spos, slen, handle,
> identifier)) {
> + return -1;
> + }
Not needed. We are heading to a while statement with "spos < pos +
ulen", so this is auto handled.
> spos = spos + slen;
> }
> -
> + if(length_check( blen, hlen, 8, handle, identifier)) {
> + return -1;
> + }
Redundant check, we will check again before accessing the element.
> pos = pos + (blen - hlen - 8);> }
> -
> return 0;
> }
>
