nodegit / nodegit/nodegit

NodeGit performs a double free on string arrays if libgit2 gives a non-zero error code

Offen
#1,226 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Vorherrschende Sprache
JavaScript
Sterne
5.8k
Forks
704
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

The generated native code for the following five JavaScript functions all have this problem:

Reference.list
Remote.list
Remote#getFetchRefspecs 
Remote#getPushRefspecs 
Tag.list

We have had two crashes (1, 2) in Orion thus far because of this problem. From talking to the libgit2 team, it is not easy to reproduce the error because at this point it's a thing with malloc not working out or a thing with native file I/O.

Remote.list
Please scroll to the bottom of this code snippet to see the problem.

void GitRemote::ListWorker::HandleOKCallback() {
  if (baton->error_code == GIT_OK) {
    Local<v8::Value> to;
// start convert_to_v8 block
    Local<Array> tmpArray = Nan::New<Array>(baton->out->count);
    for (unsigned int i = 0; i < baton->out->count; i++) {
      Nan::Set(tmpArray, Nan::New<Number>(i), Nan::New<String>(baton->out->strings[i]).ToLocalChecked());
    }
 
  to = tmpArray;
 // end convert_to_v8 block
    Local<v8::Value> result = to;
    Local<v8::Value> argv[2] = {
      Nan::Null(),
      result
    };
    callback->Call(2, argv);
  } else {
    if (baton->error) {
      Local<v8::Value> argv[1] = {
        Nan::Error(baton->error->message)
      };
      callback->Call(1, argv);
      if (baton->error->message)
        free((void *)baton->error->message);
      free((void *)baton->error);
    } else if (baton->error_code < 0) {
      std::queue< Local<v8::Value> > workerArguments;
      workerArguments.push(GetFromPersistent("repo"));
      bool callbackFired = false;
      while(!workerArguments.empty()) {
        Local<v8::Value> node = workerArguments.front();
        workerArguments.pop();

        if (
          !node->IsObject()
          || node->IsArray()
          || node->IsBooleanObject()
          || node->IsDate()
          || node->IsFunction()
          || node->IsNumberObject()
          || node->IsRegExp()
          || node->IsStringObject()
        ) {
          continue;
        }

        Local<v8::Object> nodeObj = node->ToObject();
        Local<v8::Value> checkValue = GetPrivate(nodeObj, Nan::New("NodeGitPromiseError").ToLocalChecked());

        if (!checkValue.IsEmpty() && !checkValue->IsNull() && !checkValue->IsUndefined()) {
          Local<v8::Value> argv[1] = {
            checkValue->ToObject()
          };
          callback->Call(1, argv);
          callbackFired = true;
          break;
        }

        Local<v8::Array> properties = nodeObj->GetPropertyNames();
        for (unsigned int propIndex = 0; propIndex < properties->Length(); ++propIndex) {
          Local<v8::String> propName = properties->Get(propIndex)->ToString();
          Local<v8::Value> nodeToQueue = nodeObj->Get(propName);
          if (!nodeToQueue->IsUndefined()) {
            workerArguments.push(nodeToQueue);
          }
        }
      }

      if (!callbackFired) {
        Local<v8::Object> err = Nan::Error("Method list has thrown an error.")->ToObject();
        err->Set(Nan::New("errno").ToLocalChecked(), Nan::New(baton->error_code));
        Local<v8::Value> argv[1] = {
          err
        };
        callback->Call(1, argv);
      }
    } else {
      callback->Call(0, NULL);
    }

    free((void*)baton->out);
  }

  free((void *)baton->out);

  delete baton;
}

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Finde die generierten nativen Implementierungen für Reference.list, Remote.list, Remote#getFetchRefspecs, Remote#getPushRefspecs und Tag.list. Beginne mit dem Pfad für Fehler ungleich null, der für Remote.list gezeigt wird, und verfolge die Zuständigkeit für baton->out; fertig ist die Aufgabe, wenn jede Funktion die Ausgabe genau einmal freigibt, sobald libgit2 einen Fehler meldet, wobei der Fehlerpfad, sofern möglich, ausgeführt wird.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
cpp, git, javascript, node.js
Bereich
backend, devtools
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Veraltet
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
42/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.