[Asterisk-code-review] chan_iax2: Prevent crash if dialing RSA-only call without outkey. (asterisk[master])

[Kevin Harwell]

Attention is currently required from: N A.
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18363 )

Change subject: chan_iax2: Prevent crash if dialing RSA-only call without outkey.
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

File channels/chan_iax2.c:

https://gerrit.asterisk.org/c/asterisk/+/18363/comment/2e53d70c_358ae230 
PS1, Line 6382: static int invalid_key(ast_aes_decrypt_key *ecx)
As it appears 'ecx' is not altered in this function this can be made const if you want.


https://gerrit.asterisk.org/c/asterisk/+/18363/comment/68b99c86_4fca3398 
PS1, Line 6384: 	int i, c = 0;
              : 	for (i = 0; i < 60; i++) {
              : 		if (ecx->rd_key[i]) {
              : 			break; /* stop if we encounter anything non-zero */
              : 		}
              : 		c += ecx->rd_key[i];
              : 	}
My eyes may deceive me but this appears to be of no affect on the function, and its result?

'c' seems to be unused, and 'ecx' is not modified in any way?


https://gerrit.asterisk.org/c/asterisk/+/18363/comment/84cd98d9_23766c9f 
PS1, Line 6391: 	if (!ecx->rd_key[0]) {
              : 		return -1;	/* if ast_aes_encrypt or ast_aes_decrypt is called, then
              : 				 * we'll crash when calling AES_encrypt or AES_decrypt */
              : 	}
As this always just checks at the [0] element then I think this can be moved to the beginning of the function to short-circuit other checks.


https://gerrit.asterisk.org/c/asterisk/+/18363/comment/0448dc0a_328c25ea 
PS1, Line 8554: 			ast_log(LOG_ERROR, "BUG! Encryption key is completely empty!\n");
              : 			return -1;
              : 		} else if (invalid_key(&p->dcx)) {
              : 			ast_log(LOG_ERROR, "BUG! Decryption key is completely empty!\n");
Should these just be 'ast_assert_return's?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18363
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: If925c3d86099ceac7f621804f2532baac5050c9a
Gerrit-Change-Number: 18363
Gerrit-PatchSet: 1
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Comment-Date: Tue, 26 Apr 2022 22:17:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220426/49da4ffc/attachment.html>