Add a semantic patch to prevent use after free.

[mansourmoufid]

Hello, thank you for this excellent project.

In the file soter_hmac.c there is a double-free.

themis/src/soter/soter_hmac.c

Lines 96 to 97 in 10b8ced

	soter_hash_destroy(hmac_ctx->hash_ctx);
	soter_hmac_cleanup(hmac_ctx);

The function soter_hmac_cleanup immediately follows soter_hash_destroy.

The first function frees hmac_ctx->hash_ctx:

themis/src/soter/openssl/soter_hash.c

Lines 132 to 140 in 10b8ced

	soter_status_t soter_hash_destroy(soter_hash_ctx_t* hash_ctx)
	{
	if (!hash_ctx) {
	return SOTER_INVALID_PARAMETER;
	}
	soter_hash_cleanup(hash_ctx);
	free(hash_ctx);
	return SOTER_SUCCESS;
	}

The second function calls the first function again, with the same argument:

themis/src/soter/soter_hmac.c

Lines 112 to 122 in 10b8ced

	soter_status_t soter_hmac_cleanup(soter_hmac_ctx_t* hmac_ctx)
	{
	if (NULL == hmac_ctx) {
	return SOTER_INVALID_PARAMETER;
	}
	memset(hmac_ctx->o_key_pad, 0, sizeof(hmac_ctx->o_key_pad));
	soter_hash_destroy(hmac_ctx->hash_ctx);
	hmac_ctx->hash_ctx = NULL;
	return SOTER_SUCCESS;
	}

This can be prevented by inserting a NULL assignment between the two function calls.

Here is a semantic patch (see Coccinelle) that does this for all such functions (not only in this specific instance, although it turns out only this one instance was a bug).

The file themis-free-functions.txt contains a list of all functions that free one of their arguments. The file null-after-free.cocci is a semantic patch that inserts a NULL assignment after every call to each of the functions in the previous file, and then attempts to remove unnecessary NULL assignments.

Apply this semantic patch with the command:

spatch --sp-file scripts/cocci/null-after-free.cocci --dir src --in-place < scripts/cocci/themis-free-functions.txt 

[vixentael]

Thank you, @eliteraspberries! This looks fantastic (especially semantic patch) 👍

Double free fixes are very timely: we did some of them during last month (#525, #535, #524). Great job!

[ilammy]

Wow! Coccinelle... That’s a tool I’ve seen used mostly in Linux kernel development. Maybe because it’s one of the few "big" C projects that warrant its use. Or because people somehow rely more on their IDEs for refactoring.

[ilammy]

The changes look sound. Sign me up for merging this.

@eliteraspberries, thank you for your contribution! ❤️

Double kudos for showing an example of Coccinelle. I hope we’d make a good use of this amazing tool for future refactoring.

[ilammy]

P.S. Some notes if anyone wants to apply the semantic patch from source. On my Linux box Coccinelle from repos works without an issue. On macOS Coccinelle from Homebrew does not like Homebrew’s Python 2 (python@2), removing it and leaving only system Python and Python 3 from Homebrew is enough to make it work.