Additional error handling for rarer states.

Ensure std::map::at is not blindly applied without checking that the key
actually exists.

Fixes crashes where u2f client on PC/browser expects channel open, but
service has been restarted.
This commit is contained in:
2019-09-08 20:27:47 +01:00
parent f8d077634e
commit 89b4b7d949
4 changed files with 20 additions and 10 deletions

View File

@@ -48,17 +48,19 @@ void Controller::handleTransaction(const U2FMessage& msg) {
lastMessage = chrono::system_clock::now(); lastMessage = chrono::system_clock::now();
auto opChannel = msg.cid; auto opChannelID = msg.cid;
if (msg.cmd == U2FHID_INIT) { if (msg.cmd == U2FHID_INIT) {
opChannel = nextChannel(); opChannelID = nextChannel();
auto channel = Channel{ opChannel }; auto channel = Channel{ opChannelID };
try { try {
channels.emplace(opChannel, channel); // In case of wrap-around replace existing one channels.emplace(opChannelID, channel); // In case of wrap-around replace existing one
} catch (...) { } catch (...) {
channels.insert(make_pair(opChannel, channel)); channels.insert(make_pair(opChannelID, channel));
} }
} else if (channels.find(opChannelID) == channels.end()) {
U2FMessage::error(opChannelID, ERR_CHANNEL_BUSY);
return;
} }
#ifdef DEBUG_MSGS #ifdef DEBUG_MSGS
@@ -66,7 +68,7 @@ void Controller::handleTransaction(const U2FMessage& msg) {
clog << "cid: " << msg.cid << ", cmd: " << static_cast<unsigned int>(msg.cmd) << endl; clog << "cid: " << msg.cid << ", cmd: " << static_cast<unsigned int>(msg.cmd) << endl;
#endif #endif
channels.at(opChannel).handle(msg); channels.at(opChannelID).handle(msg);
} }
uint32_t Controller::nextChannel() { uint32_t Controller::nextChannel() {

View File

@@ -186,7 +186,10 @@ FILE* initHTML(FILE* fPtr, const string& title) {
void closeHTML(FILE* fPtr) { void closeHTML(FILE* fPtr) {
fprintf(fPtr, "\t</body>\n" fprintf(fPtr, "\t</body>\n"
"</html>"); "</html>");
fclose(fPtr); int successCode = fclose(fPtr);
if (successCode != 0)
cerr << "File closing error: " << errno << endl;
} }
#endif #endif

View File

@@ -60,7 +60,7 @@ void U2F_Authenticate_APDU::respond(const uint32_t channelID) const {
if (Storage::appParams.find(keyHB) == Storage::appParams.end()) { if (Storage::appParams.find(keyHB) == Storage::appParams.end()) {
// Respond with error code - key handle doesn't exist in storage // Respond with error code - key handle doesn't exist in storage
cerr << "Invalid key handle" << endl; cerr << "Invalid key handle" << endl;
this->error(channelID, SW_WRONG_DATA); this->error(channelID, APDU_STATUS::SW_WRONG_DATA);
return; return;
} }

View File

@@ -81,7 +81,12 @@ shared_ptr<U2F_Msg_CMD> U2F_Msg_CMD::generate(const U2FMessage& uMsg) {
const uint32_t cBCount = data.size(); const uint32_t cBCount = data.size();
auto startPtr = data.begin(), endPtr = data.end(); auto startPtr = data.begin(), endPtr = data.end();
if (usesData.at(cmd.ins) || data.size() > 3) { const auto cmdUsesData = usesData.find(cmd.ins);
if (cmdUsesData == usesData.end()) {
U2F_Msg_CMD::error(uMsg.cid, APDU_STATUS::SW_INS_NOT_SUPPORTED);
throw runtime_error{ "Unknown instruction: unsure if uses data" };
} else if (cmdUsesData->second || data.size() > 3) {
if (cBCount == 0) { if (cBCount == 0) {
U2F_Msg_CMD::error(uMsg.cid, APDU_STATUS::SW_WRONG_LENGTH); U2F_Msg_CMD::error(uMsg.cid, APDU_STATUS::SW_WRONG_LENGTH);
throw runtime_error{ "Invalid command - should have attached data" }; throw runtime_error{ "Invalid command - should have attached data" };