CBBs are in an undefined state after an operation failed.
Our CBB patterns do not make it safe to use a CBB after any operation
failed. Suppose one does:
int add_to_cbb(CBB *cbb) {
CBB child;
return CBB_add_u8(cbb, 1) &&
CBB_add_u8_length_prefixed(cbb, &child) &&
CBB_add_u8(&child, 2) &&
/* Flush |cbb| before |child| goes out of scoped. */
CBB_flush(cbb);
}
If one of the earlier operations fails, any attempt to use |cbb| (except
CBB_cleanup) would hit a memory error. Doing this would be a bug anyway,
since the CBB would be in an undefined state anyway (wrote only half my
object), but the memory error is bad manners.
Officially document that using a CBB after failure is illegal and, to
avoid the memory error, set a poison bit on the cbb_buffer_st to prevent
all future operations. In theory we could make failure +
CBB_discard_child work, but this is not very useful and would require a
more complex CBB pattern.
Change-Id: I4303ee1c326785849ce12b5f7aa8bbde6b95d2ec
Reviewed-on: https://boringssl-review.googlesource.com/8840
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h
index c24281a..68138bc 100644
--- a/include/openssl/bytestring.h
+++ b/include/openssl/bytestring.h
@@ -242,7 +242,8 @@
* not be used again.
*
* If one needs to force a length prefix to be written out because a |CBB| is
- * going out of scope, use |CBB_flush|. */
+ * going out of scope, use |CBB_flush|. If an operation on a |CBB| fails, it is
+ * in an undefined state and must not be used except to call |CBB_cleanup|. */
struct cbb_buffer_st {
uint8_t *buf;
@@ -250,6 +251,8 @@
size_t cap; /* The size of buf. */
char can_resize; /* One iff |buf| is owned by this object. If not then |buf|
cannot be resized. */
+ char error; /* One iff there was an error writing to this CBB. All future
+ operations will fail. */
};
struct cbb_st {
