Closed w3bb closed 1 year ago
Can you check the current signing method in pytoast first?
It isn't wrapped properly.
{
[(changes)]
}
is now
{
"revision": 69
"changes": [(changes)]
}
The cryptography is more or less useless without that revision field included -- attackers can reorder the revisions and cause corruption or reintroduce vulnerable files. If it is not the new format, signing should not be used. Here's the approach murse uses:
func (c *Client) GetRevision(v int) ([]Change, error) {
object, err := c.getRead(c.repo + "revisions/" + strconv.Itoa(v))
if err != nil {
return nil, err
}
if c.key != nil {
sig, err := c.getRead(c.repo + "revisions/" + strconv.Itoa(v) + ".sig")
if err != nil {
return nil, err
}
ok := verifyContents(c.key, object, sig)
if !ok {
return nil, errors.New("failed cryptographic check")
}
}
var wrappedRevision Revision
err = json.Unmarshal(object, &wrappedRevision)
if err != nil {
if c.key == nil {
var unwrappedRevision []Change
err = json.Unmarshal(object, &unwrappedRevision)
if err != nil {
return nil, err
}
return unwrappedRevision, nil
}
return nil, err
}
On the signing branch i'm sure I've introduced this exactly.
def fetch_revisions(url, first, last, verif):
revisions = []
for x in range(first + 1, last + 1):
if not (x < 0):
rev = httpx.get(url + "revisions/" + str(x), headers={'user-agent': user_agent}, follow_redirects=True)
sig = httpx.get(url + "revisions/" + str(x) + ".sig", headers={'user-agent': user_agent},
follow_redirects=True)
latest_hash = SHA256.new(rev.read())
try:
verif.verify(latest_hash, sig.read())
except:
errorMsg = QMessageBox()
errorMsg.setWindowTitle("Toast Meditation")
errorMsg.setText("Error: Revision Signature is invalid!\nThis means that there's some issue on the "
"server, and it can't verify that the data hasn't been tampered with.\nCheck the "
"troubleshooting channel's pins.")
errorMsg.exec_()
exit()
revisions.append(json.loads(rev.text))
return revisions
You aren't checking here for if the revision is correct, and you aren't returning the contents of Change, just a dump of the JSON itself.
revisions = fetch_revisions(url, installed_revision, latest_revision, verif)
changes = replay_changes(revisions)
Then you pass it as of it were just a list of Change instead of a proper wrapped object.
I mean on pytoast I've implemented the revision field.
The plan for murse is to have a transitional release that supports both both the unwrapped Change array, and the new wrapped array with the revision number. I think this approach would work well for OFJam where people are slow to update (if they update at all.) Hopefully once we fully transition murse and OFJam can remove support for the old schema at the same time.
The only change that needs to be done is to check if the JSON object is wrapped under .changes if signatures are disabled. Otherwise, if enabled, it must be wrapped.
I'm waiting to hear back about this before I push the transitional release for murse. I did mention this in the guild but this probably makes more sense as an issue.