Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libexpr: Fix read out-of-bound on the heap #5011

Merged
merged 1 commit into from
Jul 15, 2021

Conversation

Pamplemousse
Copy link
Member

@Pamplemousse Pamplemousse commented Jul 13, 2021

  • When s contains \r\n, len does not get decremented enough, and so more looping than necessary happens;
  • I replaced the while loop with a for because len is a size_t, thus decrementing it if its value is 0 would not stop the loop.

cc @regnat

Signed-off-by: Pamplemousse [email protected]

@edolstra
Copy link
Member

This breaks some tests because it changes column/line numbers:

    evaluating lang/eval-okay-getattrpos-functionargs.nix (should succeed)
    1c1
    < { column = 1; file = "eval-okay-getattrpos-functionargs.nix"; line = 1; }
    ---
    > { column = 11; file = "eval-okay-getattrpos-functionargs.nix"; line = 2; }

@Pamplemousse
Copy link
Member Author

Pamplemousse commented Jul 14, 2021

Good thing there are tests, the conditional in the loop was broken 😅

@edolstra edolstra merged commit 77d5b37 into NixOS:master Jul 15, 2021
@Pamplemousse Pamplemousse deleted the fix_adjustLoc branch July 15, 2021 16:07
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-fellowship-fuzzing-nix-2/14228/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-16/14379/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants