Fix Security Audit issue OPGP-#01

- Fix buffer overflow in pinentry buffer
- Add dedicated variable to manage the pin length
This commit is contained in:
Charles-Edouard de la Vergne 2024-03-13 11:48:49 +01:00
parent 6983d8cbb8
commit c3c0fde6fc
No known key found for this signature in database
GPG Key ID: F12296941B7BB9C6
3 changed files with 28 additions and 35 deletions

View File

@ -270,7 +270,8 @@ struct gpg_v_state_s {
/* ux menus */
char menu[112];
unsigned char ux_pinentry[12];
unsigned char ux_pinentry[GPG_MAX_PW_LENGTH];
unsigned char ux_pinLen;
unsigned int ux_key;
unsigned int ux_type;

View File

@ -354,8 +354,7 @@ unsigned int ui_pinentry_predisplay(const bagl_element_t *element) {
}
} else if (element->component.userid == 2) {
unsigned int i;
G_gpg_vstate.menu[0] = ' ';
for (i = 1; i < G_gpg_vstate.ux_pinentry[0]; i++) {
for (i = 0; i < G_gpg_vstate.ux_pinLen; i++) {
G_gpg_vstate.menu[i] = '*';
}
G_gpg_vstate.menu[i] = C_pin_digit[G_gpg_vstate.ux_pinentry[i]];
@ -372,8 +371,8 @@ unsigned int ui_pinentry_predisplay(const bagl_element_t *element) {
void ui_menu_pinentry_display(unsigned int value) {
if (value == 0) {
memset(G_gpg_vstate.ux_pinentry, 0, sizeof(G_gpg_vstate.ux_pinentry));
G_gpg_vstate.ux_pinentry[0] = 1;
G_gpg_vstate.ux_pinentry[1] = 5;
G_gpg_vstate.ux_pinLen = 0;
G_gpg_vstate.ux_pinentry[0] = 5;
}
UX_DISPLAY(ui_pinentry_action, (void *) ui_pinentry_predisplay);
}
@ -382,7 +381,7 @@ static void validate_pin() {
unsigned int offset, len, sw = SW_UNKNOWN;
gpg_pin_t *pin;
for (offset = 1; offset <= G_gpg_vstate.ux_pinentry[0]; offset++) {
for (offset = 0; offset <= G_gpg_vstate.ux_pinLen; offset++) {
G_gpg_vstate.menu[offset] = C_pin_digit[G_gpg_vstate.ux_pinentry[offset]];
}
@ -390,8 +389,8 @@ static void validate_pin() {
pin = gpg_pin_get_pin(G_gpg_vstate.io_p2);
sw = gpg_pin_check(pin,
G_gpg_vstate.io_p2,
(unsigned char *) (G_gpg_vstate.menu + 1),
G_gpg_vstate.ux_pinentry[0]);
(unsigned char *) G_gpg_vstate.menu,
G_gpg_vstate.ux_pinLen);
gpg_io_discard(1);
gpg_io_insert_u16(sw);
gpg_io_do(IO_RETURN_AFTER_TX);
@ -408,8 +407,8 @@ static void validate_pin() {
if (G_gpg_vstate.io_ins == INS_CHANGE_REFERENCE_DATA) {
if (G_gpg_vstate.io_p1 <= 2) {
gpg_io_insert_u8(G_gpg_vstate.ux_pinentry[0]);
gpg_io_insert((unsigned char *) (G_gpg_vstate.menu + 1), G_gpg_vstate.ux_pinentry[0]);
gpg_io_insert_u8(G_gpg_vstate.ux_pinLen);
gpg_io_insert((unsigned char *) G_gpg_vstate.menu, G_gpg_vstate.ux_pinLen);
G_gpg_vstate.io_p1++;
}
if (G_gpg_vstate.io_p1 == 3) {
@ -455,7 +454,7 @@ static void validate_pin() {
unsigned int ui_pinentry_action_button(unsigned int button_mask, unsigned int button_mask_counter) {
UNUSED(button_mask_counter);
unsigned int offset = G_gpg_vstate.ux_pinentry[0];
unsigned int offset = G_gpg_vstate.ux_pinLen;
char digit;
switch (button_mask) {
@ -480,9 +479,8 @@ unsigned int ui_pinentry_action_button(unsigned int button_mask, unsigned int bu
digit = C_pin_digit[G_gpg_vstate.ux_pinentry[offset]];
// next digit
if ((digit >= '0') && (digit <= '9')) {
offset++;
G_gpg_vstate.ux_pinentry[0] = offset;
if (offset == GPG_MAX_PW_LENGTH + 1) {
G_gpg_vstate.ux_pinLen = ++offset;
if (offset == GPG_MAX_PW_LENGTH) {
validate_pin();
} else {
G_gpg_vstate.ux_pinentry[offset] = 5;
@ -491,15 +489,13 @@ unsigned int ui_pinentry_action_button(unsigned int button_mask, unsigned int bu
}
// cancel digit
else if (digit == '<') {
if (offset > 1) {
offset--;
G_gpg_vstate.ux_pinentry[0] = offset;
if (offset > 0) {
G_gpg_vstate.ux_pinLen--;
}
ui_menu_pinentry_display(1);
}
// validate pin
else if (digit == 'V') {
G_gpg_vstate.ux_pinentry[0] = offset - 1;
validate_pin();
}
// cancel input without check

View File

@ -287,8 +287,7 @@ unsigned int ui_pinentry_predisplay(const bagl_element_t *element) {
}
} else if (element->component.userid == 2) {
unsigned int i;
G_gpg_vstate.menu[0] = ' ';
for (i = 1; i < G_gpg_vstate.ux_pinentry[0]; i++) {
for (i = 0; i < G_gpg_vstate.ux_pinLen; i++) {
G_gpg_vstate.menu[i] = '*';
}
G_gpg_vstate.menu[i] = C_pin_digit[G_gpg_vstate.ux_pinentry[i]];
@ -305,8 +304,8 @@ unsigned int ui_pinentry_predisplay(const bagl_element_t *element) {
void ui_menu_pinentry_display(unsigned int value) {
if (value == 0) {
memset(G_gpg_vstate.ux_pinentry, 0, sizeof(G_gpg_vstate.ux_pinentry));
G_gpg_vstate.ux_pinentry[0] = 1;
G_gpg_vstate.ux_pinentry[1] = 5;
G_gpg_vstate.ux_pinLen = 0;
G_gpg_vstate.ux_pinentry[0] = 5;
}
UX_DISPLAY(ui_pinentry_action, (void *) ui_pinentry_predisplay);
}
@ -315,7 +314,7 @@ static void validate_pin() {
unsigned int offset, len, sw = SW_UNKNOWN;
gpg_pin_t *pin;
for (offset = 1; offset <= G_gpg_vstate.ux_pinentry[0]; offset++) {
for (offset = 0; offset <= G_gpg_vstate.ux_pinLen; offset++) {
G_gpg_vstate.menu[offset] = C_pin_digit[G_gpg_vstate.ux_pinentry[offset]];
}
@ -323,8 +322,8 @@ static void validate_pin() {
pin = gpg_pin_get_pin(G_gpg_vstate.io_p2);
sw = gpg_pin_check(pin,
G_gpg_vstate.io_p2,
(unsigned char *) (G_gpg_vstate.menu + 1),
G_gpg_vstate.ux_pinentry[0]);
(unsigned char *) G_gpg_vstate.menu,
G_gpg_vstate.ux_pinLen);
gpg_io_discard(1);
gpg_io_insert_u16(sw);
gpg_io_do(IO_RETURN_AFTER_TX);
@ -341,8 +340,8 @@ static void validate_pin() {
if (G_gpg_vstate.io_ins == INS_CHANGE_REFERENCE_DATA) {
if (G_gpg_vstate.io_p1 <= 2) {
gpg_io_insert_u8(G_gpg_vstate.ux_pinentry[0]);
gpg_io_insert((unsigned char *) (G_gpg_vstate.menu + 1), G_gpg_vstate.ux_pinentry[0]);
gpg_io_insert_u8(G_gpg_vstate.ux_pinLen);
gpg_io_insert((unsigned char *) G_gpg_vstate.menu, G_gpg_vstate.ux_pinLen);
G_gpg_vstate.io_p1++;
}
if (G_gpg_vstate.io_p1 == 3) {
@ -388,7 +387,7 @@ static void validate_pin() {
unsigned int ui_pinentry_action_button(unsigned int button_mask, unsigned int button_mask_counter) {
UNUSED(button_mask_counter);
unsigned int offset = G_gpg_vstate.ux_pinentry[0];
unsigned int offset = G_gpg_vstate.ux_pinLen;
char digit;
switch (button_mask) {
@ -413,9 +412,8 @@ unsigned int ui_pinentry_action_button(unsigned int button_mask, unsigned int bu
digit = C_pin_digit[G_gpg_vstate.ux_pinentry[offset]];
// next digit
if ((digit >= '0') && (digit <= '9')) {
offset++;
G_gpg_vstate.ux_pinentry[0] = offset;
if (offset == GPG_MAX_PW_LENGTH + 1) {
G_gpg_vstate.ux_pinLen = ++offset;
if (offset == GPG_MAX_PW_LENGTH) {
validate_pin();
} else {
G_gpg_vstate.ux_pinentry[offset] = 5;
@ -424,15 +422,13 @@ unsigned int ui_pinentry_action_button(unsigned int button_mask, unsigned int bu
}
// cancel digit
else if (digit == '<') {
if (offset > 1) {
offset--;
G_gpg_vstate.ux_pinentry[0] = offset;
if (offset > 0) {
G_gpg_vstate.ux_pinLen--;
}
ui_menu_pinentry_display(1);
}
// validate pin
else if (digit == 'V') {
G_gpg_vstate.ux_pinentry[0] = offset - 1;
validate_pin();
}
// cancel input without check