On Wed, May 02, 2012 at 04:55:06AM +0100, Ben Hutchings wrote: > This validation is crap; you're not accounting for the size of the node > or invalid lengths. Try:
Good catch. > Isn't this slightly too restrictive? The '& 0xff' results in many > non-ASCII characters aliasing hex digits and potentially causes a > variable to be validated as if it was special. I would think the > correct condition is: Mm. Yeah, I guess that should probably be permitted. > if (var->VariableName[i] > 127 || > hex_to_bin(var->VariableName[i]) < 0) > > Presumably the variable should also be ignored if there are any more > characters after the 4 hex digits? The spec doesn't permit any such names, so I don't think it makes any difference in practice. But in theory, we should probably either explicitly permit or reject them rather than treating them as if they're boot variables. > > + /* A valid entry must be at least 6 bytes */ > > + if (len < 6) > > + return false; > > Surely 8 bytes - otherwise you don't even have space for the > description's null terminator. Yes, I guess that could trip things up. > > + filepathlength = buffer[4] | buffer[5] << 8; > > + > > + /* > > + * There's no stored length for the description, so it has to be > > + * found by hand > > + */ > > + desclength = utf16_strsize((efi_char16_t *)(buffer + 6), len) + 2; > [...] > > Second argument should be len - 6. Sure. -- Matthew Garrett | [email protected] -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
