Commit b05f9637 authored by npzacs's avatar npzacs
Browse files

Check cache memory size before reading.

TOCTOU between data size check and read: attacker could replace cache
file with larger one and cause buffer overflow.
parent 9f3f7405
...@@ -473,7 +473,7 @@ int cache_save(const char *name, uint32_t version, const void *data, uint32_t le ...@@ -473,7 +473,7 @@ int cache_save(const char *name, uint32_t version, const void *data, uint32_t le
return result; return result;
} }
int cache_get(const char *name, uint32_t *version, uint32_t *len, void *buf) int cache_get(const char *name, uint32_t *version, uint32_t *len, void *buf, size_t buf_size)
{ {
int result = 0; int result = 0;
char *file = _cache_file(name); char *file = _cache_file(name);
...@@ -491,6 +491,7 @@ int cache_get(const char *name, uint32_t *version, uint32_t *len, void *buf) ...@@ -491,6 +491,7 @@ int cache_get(const char *name, uint32_t *version, uint32_t *len, void *buf)
if (fread(version, 1, 4, fp) == 4 && if (fread(version, 1, 4, fp) == 4 &&
(!len || fread(len, 1, 4, fp) == 4) && (!len || fread(len, 1, 4, fp) == 4) &&
(!len || (size_t)*len <= buf_size) &&
(!buf || fread(buf, 1, *len, fp) == *len)) { (!buf || fread(buf, 1, *len, fp) == *len)) {
BD_DEBUG(DBG_FILE, "Read %d bytes from %s, version %d\n", 4 + (len ? 4 : 0) + (buf ? *len : 0), file, *version); BD_DEBUG(DBG_FILE, "Read %d bytes from %s, version %d\n", 4 + (len ? 4 : 0) + (buf ? *len : 0), file, *version);
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "util/attributes.h" #include "util/attributes.h"
#include <stdint.h> #include <stdint.h>
#include <stddef.h> /* size_t */
/* struct holding a digit and key pair for <ENTRY NUMBER> - <ENTRY> entries */ /* struct holding a digit and key pair for <ENTRY NUMBER> - <ENTRY> entries */
typedef struct digit_key_pair_t digit_key_pair; typedef struct digit_key_pair_t digit_key_pair;
...@@ -129,7 +130,7 @@ BD_PRIVATE int keycache_save(const char *type, const uint8_t *disc_id, ...@@ -129,7 +130,7 @@ BD_PRIVATE int keycache_save(const char *type, const uint8_t *disc_id,
BD_PRIVATE int keycache_find(const char *type, const uint8_t *disc_id, BD_PRIVATE int keycache_find(const char *type, const uint8_t *disc_id,
uint8_t *key, unsigned int len); uint8_t *key, unsigned int len);
BD_PRIVATE int cache_get(const char *name, uint32_t *version, uint32_t *len, void *buf); /* use buf=NULL to get version and size */ BD_PRIVATE int cache_get(const char *name, uint32_t *version, uint32_t *len, void *buf, size_t buf_size); /* use buf=NULL to get version and size */
BD_PRIVATE int cache_save(const char *name, uint32_t version, const void *data, uint32_t len); BD_PRIVATE int cache_save(const char *name, uint32_t version, const void *data, uint32_t len);
BD_PRIVATE int cache_remove(const char *name); BD_PRIVATE int cache_remove(const char *name);
......
...@@ -175,14 +175,14 @@ static void _update_rl(MKB *mkb) ...@@ -175,14 +175,14 @@ static void _update_rl(MKB *mkb)
uint32_t cache_version; uint32_t cache_version;
size_t rl_len; size_t rl_len;
if (!cache_get("drl", &cache_version, NULL, NULL) || cache_version < version) { if (!cache_get("drl", &cache_version, NULL, NULL, 0) || cache_version < version) {
const uint8_t *version_rec = mkb_type_and_version_record(mkb); const uint8_t *version_rec = mkb_type_and_version_record(mkb);
const uint8_t *drl_rec = mkb_drive_revokation_entries(mkb, &rl_len); const uint8_t *drl_rec = mkb_drive_revokation_entries(mkb, &rl_len);
if (drl_rec && rl_len > 8) { if (drl_rec && rl_len > 8) {
_save_rl("drl", version, version_rec, drl_rec, rl_len); _save_rl("drl", version, version_rec, drl_rec, rl_len);
} }
} }
if (!cache_get("hrl", &cache_version, NULL, NULL) || cache_version < version) { if (!cache_get("hrl", &cache_version, NULL, NULL, 0) || cache_version < version) {
const uint8_t *version_rec = mkb_type_and_version_record(mkb); const uint8_t *version_rec = mkb_type_and_version_record(mkb);
const uint8_t *hrl_rec = mkb_host_revokation_entries(mkb, &rl_len); const uint8_t *hrl_rec = mkb_host_revokation_entries(mkb, &rl_len);
if (hrl_rec && rl_len > 8) { if (hrl_rec && rl_len > 8) {
...@@ -1294,11 +1294,11 @@ static AACS_RL_ENTRY *_get_rl(const char *type, int *num_records, int *mkbv) ...@@ -1294,11 +1294,11 @@ static AACS_RL_ENTRY *_get_rl(const char *type, int *num_records, int *mkbv)
*num_records = *mkbv = 0; *num_records = *mkbv = 0;
cache_get(type, &version, &len, NULL); cache_get(type, &version, &len, NULL, 0);
if (version > 0 && len > 24) { if (version > 0 && len > 24) {
data = malloc(len); data = malloc(len);
if (cache_get(type, &version, &len, data) && len > 24) { if (cache_get(type, &version, &len, data, len) && len > 24) {
if (_rl_verify_signature(data, len)) { if (_rl_verify_signature(data, len)) {
*mkbv = version; *mkbv = version;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment