Fix a bug in MDP_IDENTITY request handling

The LOCK and UNLOCK by PIN requests were acting on the supplied PIN
and also the empty PIN, due to a bug in ob_get_str_ptr(), which returned
an empty string instead of NULL after reaching the end of the string
list.
This commit is contained in:
Andrew Bettison 2018-03-09 16:54:02 +10:30
parent 2333a116f3
commit 8242ca0a00
2 changed files with 37 additions and 42 deletions

View File

@ -413,17 +413,15 @@ void _ob_append_packed_ui64(struct __sourceloc __whence, struct overlay_buffer *
// make sure a range of bytes is valid for reading // make sure a range of bytes is valid for reading
static int test_offset(struct overlay_buffer *b, size_t length) static int test_offset(struct overlay_buffer *b, size_t length)
{ {
if (b->position + length > b->sizeLimit) assert(length != 0);
return -1; size_t off = b->position + length;
if (b->position + length > b->allocSize) return off <= b->sizeLimit && off <= b->allocSize;
return -1;
return 0;
} }
// next byte without advancing // next byte without advancing
int ob_peek(struct overlay_buffer *b) int ob_peek(struct overlay_buffer *b)
{ {
if (test_offset(b, 1)) if (!test_offset(b, 1))
return -1; return -1;
return b->bytes[b->position]; return b->bytes[b->position];
} }
@ -437,8 +435,8 @@ void ob_skip(struct overlay_buffer *b, unsigned n)
const char *ob_get_str_ptr(struct overlay_buffer *b) const char *ob_get_str_ptr(struct overlay_buffer *b)
{ {
const char *ret = (const char*)(b->bytes + b->position); const char *ret = (const char*)(b->bytes + b->position);
off_t ofs=0; off_t ofs = 0;
while (test_offset(b, ofs)==0){ while (test_offset(b, ofs + 1)) {
if (ret[ofs]=='\0'){ if (ret[ofs]=='\0'){
b->position+=ofs+1; b->position+=ofs+1;
return ret; return ret;
@ -450,7 +448,7 @@ const char *ob_get_str_ptr(struct overlay_buffer *b)
int ob_get_bytes(struct overlay_buffer *b, unsigned char *buff, size_t len) int ob_get_bytes(struct overlay_buffer *b, unsigned char *buff, size_t len)
{ {
if (test_offset(b, len)) if (!test_offset(b, len))
return -1; return -1;
bcopy(b->bytes + b->position, buff, len); bcopy(b->bytes + b->position, buff, len);
b->position+=len; b->position+=len;
@ -459,7 +457,7 @@ int ob_get_bytes(struct overlay_buffer *b, unsigned char *buff, size_t len)
unsigned char * ob_get_bytes_ptr(struct overlay_buffer *b, size_t len) unsigned char * ob_get_bytes_ptr(struct overlay_buffer *b, size_t len)
{ {
if (test_offset(b, len)) if (!test_offset(b, len))
return NULL; return NULL;
unsigned char *ret = b->bytes + b->position; unsigned char *ret = b->bytes + b->position;
b->position+=len; b->position+=len;
@ -468,7 +466,7 @@ unsigned char * ob_get_bytes_ptr(struct overlay_buffer *b, size_t len)
uint32_t ob_get_ui32(struct overlay_buffer *b) uint32_t ob_get_ui32(struct overlay_buffer *b)
{ {
if (test_offset(b, 4)) if (!test_offset(b, 4))
return 0xFFFFFFFF; // ... unsigned return 0xFFFFFFFF; // ... unsigned
uint32_t ret = (unsigned)b->bytes[b->position] << 24 uint32_t ret = (unsigned)b->bytes[b->position] << 24
| b->bytes[b->position +1] << 16 | b->bytes[b->position +1] << 16
@ -480,7 +478,7 @@ uint32_t ob_get_ui32(struct overlay_buffer *b)
uint32_t ob_get_ui32_rv(struct overlay_buffer *b) uint32_t ob_get_ui32_rv(struct overlay_buffer *b)
{ {
if (test_offset(b, 4)) if (!test_offset(b, 4))
return 0xFFFFFFFF; // ... unsigned return 0xFFFFFFFF; // ... unsigned
uint32_t ret = b->bytes[b->position] uint32_t ret = b->bytes[b->position]
| b->bytes[b->position +1] << 8 | b->bytes[b->position +1] << 8
@ -492,7 +490,7 @@ uint32_t ob_get_ui32_rv(struct overlay_buffer *b)
uint64_t ob_get_ui64(struct overlay_buffer *b) uint64_t ob_get_ui64(struct overlay_buffer *b)
{ {
if (test_offset(b, 8)) if (!test_offset(b, 8))
return 0xFFFFFFFF; // ... unsigned return 0xFFFFFFFF; // ... unsigned
uint64_t ret = (uint64_t)b->bytes[b->position] << 56 uint64_t ret = (uint64_t)b->bytes[b->position] << 56
| (uint64_t)b->bytes[b->position +1] << 48 | (uint64_t)b->bytes[b->position +1] << 48
@ -508,7 +506,7 @@ uint64_t ob_get_ui64(struct overlay_buffer *b)
uint64_t ob_get_ui64_rv(struct overlay_buffer *b) uint64_t ob_get_ui64_rv(struct overlay_buffer *b)
{ {
if (test_offset(b, 8)) if (!test_offset(b, 8))
return 0xFFFFFFFF; // ... unsigned return 0xFFFFFFFF; // ... unsigned
uint64_t ret = (uint64_t)b->bytes[b->position] uint64_t ret = (uint64_t)b->bytes[b->position]
| (uint64_t)b->bytes[b->position +1] << 8 | (uint64_t)b->bytes[b->position +1] << 8
@ -524,7 +522,7 @@ uint64_t ob_get_ui64_rv(struct overlay_buffer *b)
uint16_t ob_get_ui16(struct overlay_buffer *b) uint16_t ob_get_ui16(struct overlay_buffer *b)
{ {
if (test_offset(b, 2)) if (!test_offset(b, 2))
return 0xFFFF; // ... unsigned return 0xFFFF; // ... unsigned
uint16_t ret = b->bytes[b->position] << 8 uint16_t ret = b->bytes[b->position] << 8
| b->bytes[b->position +1]; | b->bytes[b->position +1];
@ -534,7 +532,7 @@ uint16_t ob_get_ui16(struct overlay_buffer *b)
uint16_t ob_get_ui16_rv(struct overlay_buffer *b) uint16_t ob_get_ui16_rv(struct overlay_buffer *b)
{ {
if (test_offset(b, 2)) if (!test_offset(b, 2))
return 0xFFFF; // ... unsigned return 0xFFFF; // ... unsigned
uint16_t ret = b->bytes[b->position] uint16_t ret = b->bytes[b->position]
| b->bytes[b->position +1] << 8; | b->bytes[b->position +1] << 8;
@ -574,7 +572,7 @@ uint64_t ob_get_packed_ui64(struct overlay_buffer *b)
int ob_get(struct overlay_buffer *b) int ob_get(struct overlay_buffer *b)
{ {
if (test_offset(b, 1)) if (!test_offset(b, 1))
return -1; return -1;
return b->bytes[b->position++]; return b->bytes[b->position++];
} }

View File

@ -1235,20 +1235,16 @@ static int mdp_process_identity_request(struct socket_address *client, struct md
switch (request.type){ switch (request.type){
case TYPE_PIN: case TYPE_PIN:
{ {
while(1){ const char *pin;
const char *pin = ob_get_str_ptr(payload); while ((pin = ob_get_str_ptr(payload)))
if (!pin)
break;
keyring_release_identities_by_pin(keyring, pin); keyring_release_identities_by_pin(keyring, pin);
}
} }
break; break;
case TYPE_SID: case TYPE_SID:
while(1){ {
const sid_t *sid=(const sid_t*)ob_get_bytes_ptr(payload,SID_SIZE); const sid_t *sid;
if (sid==NULL) while ((sid = (const sid_t *)ob_get_bytes_ptr(payload, SID_SIZE)))
break; keyring_release_subscriber(keyring, sid);
keyring_release_subscriber(keyring, sid);
} }
break; break;
default: default:
@ -1257,20 +1253,20 @@ static int mdp_process_identity_request(struct socket_address *client, struct md
} }
break; break;
case ACTION_UNLOCK: case ACTION_UNLOCK:
{ switch (request.type){
if (request.type!=TYPE_PIN){ case TYPE_PIN:
{
unsigned unlock_count=0;
const char *pin;
while ((pin = ob_get_str_ptr(payload)))
unlock_count += keyring_enter_pin(keyring, pin);
if (unlock_count && directory_service)
directory_registration();
}
break;
default:
mdp_reply_error(client, header); mdp_reply_error(client, header);
return WHY("Unknown request type"); return WHY("Unknown request type");
}
unsigned unlock_count=0;
while(1){
const char *pin = ob_get_str_ptr(payload);
if (!pin)
break;
unlock_count += keyring_enter_pin(keyring, pin);
}
if (unlock_count && directory_service)
directory_registration();
} }
break; break;
default: default:
@ -1303,17 +1299,17 @@ static int mdp_search_identities(struct socket_address *client, struct mdp_heade
while(1){ while(1){
if (value_len){ if (value_len){
DEBUGF(mdprequests, "Looking for next %s tag & value", tag);
if (!keyring_find_public_tag_value(&it, tag, value, value_len)) if (!keyring_find_public_tag_value(&it, tag, value, value_len))
break; break;
DEBUGF(mdprequests, "found tag=%s value=%s", tag, value);
}else if(tag){ }else if(tag){
DEBUGF(mdprequests, "Looking for next %s tag", tag);
if (!keyring_find_public_tag(&it, tag, NULL, NULL)) if (!keyring_find_public_tag(&it, tag, NULL, NULL))
break; break;
DEBUGF(mdprequests, "found tag=%s", tag);
}else{ }else{
DEBUGF(mdprequests, "Looking for next identity");
if (!keyring_next_identity(&it)) if (!keyring_next_identity(&it))
break; break;
DEBUGF(mdprequests, "found identity slot=%u SID=%s", it.identity->slot, alloca_tohex_sid_t(it.identity->subscriber->sid));
} }
unsigned char reply_payload[1200]; unsigned char reply_payload[1200];
size_t ofs=0; size_t ofs=0;
@ -1578,6 +1574,7 @@ static void mdp_process_packet(struct socket_address *client, struct mdp_header
break; break;
case MDP_IDENTITY: case MDP_IDENTITY:
DEBUGF(mdprequests, "Processing MDP_IDENTITY from %s", alloca_socket_address(client)); DEBUGF(mdprequests, "Processing MDP_IDENTITY from %s", alloca_socket_address(client));
DEBUG_dump(mdprequests, "payload", payload->bytes, payload->sizeLimit);
mdp_process_identity_request(client, header, payload); mdp_process_identity_request(client, header, payload);
break; break;
// seach unlocked identities // seach unlocked identities