Potential undersized buffer

[dodsonmg]

In macaroon_add_third_party_caveat_raw(...), I'm wondering if the buffer for enc_cyphertext should be the same size as that for enc_plaintext?

libmacaroons/macaroons.c

Lines 342 to 352 in fb878b9

	MACAROON_API struct macaroon*
	macaroon_add_third_party_caveat_raw(const struct macaroon* N,
	const unsigned char* location, size_t location_sz,
	const unsigned char* key, size_t key_sz,
	const unsigned char* id, size_t id_sz,
	enum macaroon_returncode* err)
	{
	unsigned char new_sig[MACAROON_HASH_BYTES];
	unsigned char enc_nonce[MACAROON_SECRET_NONCE_BYTES];
	unsigned char enc_plaintext[MACAROON_SECRET_TEXT_ZERO_BYTES + MACAROON_HASH_BYTES];
	unsigned char enc_ciphertext[MACAROON_SECRET_BOX_ZERO_BYTES + MACAROON_HASH_BYTES];

The call to macaroon_secretbox(...) appears to be attempting to store data beyond the end of enc_cyphertext when SECRET_BOX_OVERHEAD is not included when initialising the buffer size.

libmacaroons/macaroons.c

Lines 384 to 390 in fb878b9

	/* now encrypt the key to give us vid */
	memmove(enc_plaintext + MACAROON_SECRET_TEXT_ZERO_BYTES, key,
	key_sz < MACAROON_HASH_BYTES ? key_sz : MACAROON_HASH_BYTES);
	if (macaroon_secretbox(N->signature.data, enc_nonce, enc_plaintext,
	MACAROON_SECRET_TEXT_ZERO_BYTES + MACAROON_HASH_BYTES,
	enc_ciphertext) < 0)

In macaroon_verify_inner_3rd(...), enc_cyphertext buffer size matches enc_plaintext:

libmacaroons/macaroons.c

Lines 717 to 730 in fb878b9

	static int
	macaroon_verify_inner_3rd(const struct macaroon_verifier* V,
	const struct caveat* C,
	const unsigned char* sig,
	const struct macaroon* TM,
	struct macaroon** MS, size_t MS_sz,
	enum macaroon_returncode* err,
	size_t* tree, size_t tree_idx)
	{
	unsigned char enc_key[MACAROON_SECRET_KEY_BYTES];
	const unsigned char *enc_nonce;
	unsigned char enc_plaintext[MACAROON_SECRET_TEXT_ZERO_BYTES + MACAROON_HASH_BYTES];
	unsigned char enc_ciphertext[MACAROON_SECRET_BOX_ZERO_BYTES + MACAROON_HASH_BYTES + SECRET_BOX_OVERHEAD];
	unsigned char vid_data[VID_NONCE_KEY_SZ];

Note: SECRET_BOX_OVERHEAD is defined as the difference between MACAROON_SECRET_TEXT_ZERO_BYTES and MACAROON_SECRET_BOX_ZERO_BYTES.

libmacaroons/macaroons.c

Lines 329 to 331 in fb878b9

	#define SECRET_BOX_OVERHEAD \
	(MACAROON_SECRET_TEXT_ZERO_BYTES \
	- MACAROON_SECRET_BOX_ZERO_BYTES)