[Tarantool-patches] [PATCH v8] base64: fix decoder output buffer overrun (reads)

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Sun Mar 14 19:23:31 MSK 2021


Hi! Thanks for the patch!

Looks good, but you check out_pos in the inner loops even
though it does not change inside of them. Consider the
diff below. But I didn't bench it. Probably the compiler
moved the check out of the loops anyway in your version.
I didn't check that either. The tests pass though.

====================
@@ -235,30 +235,41 @@ base64_decode(const char *in_base64, int in_len,
 
 	while (1)
 	{
+		if (out_pos >= out_end)
+			return out_pos - out_bin;
 		do {
-			if (in_pos == in_end || out_pos >= out_end)
+			if (in_pos == in_end)
 				return out_pos - out_bin;
 			fragment = base64_decode_value(*in_pos++);
 		} while (fragment < 0);
 		*out_pos = (fragment & 0x03f) << 2;
+
+		if (out_pos >= out_end)
+			return out_pos - out_bin;
 		do {
-			if (in_pos == in_end || out_pos >= out_end)
+			if (in_pos == in_end)
 				return out_pos - out_bin;
 			fragment = base64_decode_value(*in_pos++);
 		} while (fragment < 0);
 		*out_pos++ |= (fragment & 0x030) >> 4;
+
 		if (out_pos < out_end)
 			*out_pos = (fragment & 0x00f) << 4;
+		else
+			return out_pos - out_bin;
 		do {
-			if (in_pos == in_end || out_pos >= out_end)
+			if (in_pos == in_end)
 				return out_pos - out_bin;
 			fragment = base64_decode_value(*in_pos++);
 		} while (fragment < 0);
 		*out_pos++ |= (fragment & 0x03c) >> 2;
+
 		if (out_pos < out_end)
 			*out_pos = (fragment & 0x003) << 6;
+		else
+			return out_pos - out_bin;
 		do {
-			if (in_pos == in_end || out_pos >= out_end)
+			if (in_pos == in_end)
 				return out_pos - out_bin;
 			fragment = base64_decode_value(*in_pos++);
 		} while (fragment < 0);


More information about the Tarantool-patches mailing list